Re: [PATCH 0/8] power: supply: Add driver for Qualcomm SMBCHG

[Yassine Oudjana]

On Mon, Aug 8 2022 at 11:41:26 +03:00:00, Krzysztof Kozlowski 
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
> On 08/08/2022 10:34, Yassine Oudjana wrote:
> >  From: Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx>
> >
> >  This series adds a driver for the switch-mode battery charger found 
> > on PMICs
> >  such as PMI8994, and referred to in the vendor kernel[1] as 
> > smbcharger or
> >  SMBCHG. More details on this block can be found in the last patch 
> > message.
> >
> >  This driver currently supports the charger blocks of PMI8994 and 
> > PMI8996.
> >  PMI8950 was also to be supported, but it was dropped due to some 
> > last minute
> >  issues, to be brought back at a later time once ready.
> >
> >  The OTG regulator remains unused on devices where the charger is 
> > enabled in
> >  this series due to lack of a consumer. Applying a patch[2] adding 
> > vbus-supply
> >  to DWC3 allows it to enable the OTG regulator making USB host 
> > without
> >  external power possible.
> >
> >  [1] 
> > https://github.com/android-linux-stable/msm-3.18/blob/kernel.lnx.3.18.r34-rel/drivers/power/qpnp-smbcharger.c
> >  [2] 
> > https://lore.kernel.org/linux-usb/20200805061744.20404-1-mike.looijmans@xxxxxxxx/
>
> How is it different from PMI8998? I expect not that much, so this 
> should
> be based on existing work:
> https://lore.kernel.org/linux-arm-msm/20220706194125.1861256-1-caleb.connolly@xxxxxxxxxx/
>
> Unless they are different, but then please create common parts and
> explain the differences.
>
> Best regards,
> Krzysztof

This driver has been in slow developement for a long time before that 
one existed, which was why no initial attempt at a common driver was 
made. With that said however, I've been watching its development even 
before it was sent for review, and It seems that the hardware is 
actually quite different. For example, the original charger entirely 
lacks the type-c functionality that exists on the second gen one. There 
are a couple of similar registers like CMD_APSD (same address and 
function) CHGR_CFG2 (same/similar function, different address), but 
other than that there don't seem to be any major similarities. While I 
guess it would technically be possible to force them into one driver 
with multiple register tables and separate functions for most tasks, I 
think it would just unnecessarily complicate things. One thing that is 
common however is the secure register unlock sequence, which I have 
separated in patch 6 to allow for its use in other drivers (the fuel 
gauge block has secure registers too so it will also be used in an 
upcoming fuel gauge driver).