mbox series

[v3,0/6] power: supply: Lenovo Yoga C630 EC

Message ID 20240527-yoga-ec-driver-v3-0-327a9851dad5@linaro.org
Headers show
Series power: supply: Lenovo Yoga C630 EC | expand

Message

Dmitry Baryshkov May 27, 2024, 10:03 a.m. UTC 
This adds binding, driver and the DT support for the Lenovo Yoga C630
Embedded Controller, to provide battery information.

Support for this EC was implemented by Bjorn, who later could not work
on this driver. I've picked this patchset up and updated it following
the pending review comments.

DisplayPort support is still not a part of this patchset. It uses EC
messages to provide AltMode information rather than implementing
corresponding UCSI commands. However to have a cleaner uAPI story, the
AltMode should be handled via the same Type-C port.

Merge strategy: the driver bits depend on the platform/arm64 patch,
which adds interface for the subdrivers. I'd either ask to get that
patch merged to the immutable branch, which then can be picked up by
power/supply and USB trees or, to make life simpler, ack merging all
driver bits e.g. through USB subsystem (I'm biased here since I plan to
send more cleanups for the UCSI subsystem, which would otherwise result
in cross-subsystem conflicts).

---
Changes in v3:
- Split the driver into core and power supply drivers,
- Added UCSI driver part, handling USB connections,
- Fixed Bjorn's address in DT bindings (Brian Masney)
- Changed power-role for both ports to be "dual" per UCSI
- Link to v2: https://lore.kernel.org/linux-arm-msm/20230205152809.2233436-1-dmitry.baryshkov@linaro.org/

Changes in v2:
- Dropped DP support for now, as the bindings are in process of being
  discussed separately,
- Merged dt patch into the same patchseries,
- Removed the fixed serial number battery property,
- Fixed indentation of dt bindings example,
- Added property: reg and unevaluatedProperties to the connector
  bindings.
- Link to v1: https://lore.kernel.org/linux-arm-msm/20220810035424.2796777-1-bjorn.andersson@linaro.org/

---
Bjorn Andersson (2):
      dt-bindings: power: supply: Add Lenovo Yoga C630 EC
      arm64: dts: qcom: c630: Add Embedded Controller node

Dmitry Baryshkov (4):
      platform: arm64: add Lenovo Yoga C630 WOS EC driver
      usb: typec: ucsi: add Lenovo Yoga C630 glue driver
      power: supply: lenovo_yoga_c630_battery: add Lenovo C630 driver
      arm64: dts: qcom: sdm845: describe connections of USB/DP port

 .../bindings/power/supply/lenovo,yoga-c630-ec.yaml |  83 ++++
 arch/arm64/boot/dts/qcom/sdm845.dtsi               |  53 ++-
 .../boot/dts/qcom/sdm850-lenovo-yoga-c630.dts      |  76 ++++
 drivers/platform/arm64/Kconfig                     |  14 +
 drivers/platform/arm64/Makefile                    |   1 +
 drivers/platform/arm64/lenovo-yoga-c630.c          | 279 ++++++++++++
 drivers/power/supply/Kconfig                       |   9 +
 drivers/power/supply/Makefile                      |   1 +
 drivers/power/supply/lenovo_yoga_c630_battery.c    | 476 +++++++++++++++++++++
 drivers/usb/typec/ucsi/Kconfig                     |   9 +
 drivers/usb/typec/ucsi/Makefile                    |   1 +
 drivers/usb/typec/ucsi/ucsi_yoga_c630.c            | 189 ++++++++
 include/linux/platform_data/lenovo-yoga-c630.h     |  42 ++
 13 files changed, 1232 insertions(+), 1 deletion(-)
---
base-commit: 8314289a8d50a4e05d8ece1ae0445a3b57bb4d3b
change-id: 20240527-yoga-ec-driver-76fd7f5ddae8

Best regards,

Comments

Dmitry Baryshkov May 27, 2024, 11:15 p.m. UTC | #1 
On Mon, May 27, 2024 at 02:26:36PM +0200, Oliver Neukum wrote:
> On 27.05.24 12:03, Dmitry Baryshkov wrote:
> 
> Hi,
> 
> > +struct yoga_c630_psy {
> > +	struct yoga_c630_ec *ec;
> > +	struct device *dev;
> > +	struct device_node *of_node;
> > +	struct notifier_block nb;
> > +	struct mutex lock;
> > +
> > +	struct power_supply *adp_psy;
> > +	struct power_supply *bat_psy;
> > +
> > +	unsigned long last_status_update;
> > +
> > +	bool adapter_online;
> > +
> > +	bool unit_mA;
> > +
> > +	unsigned int scale;
> 
> why do you store unit_mA and scale? This looks redundant and like a source
> of confusion to me.

Here we just followed the AML code in ACPI tables. The unit_mA is a
returned from the_BIX method, the 'scale' is used internally in the DSDT.
If you think that it's better, I can change all '* scale * 1000' to
'if unit_mA then foo = bar * 10000 else foo = bar * 1000'.
Dmitry Baryshkov May 28, 2024, 9:04 a.m. UTC | #2 
On Tue, 28 May 2024 at 11:43, Oliver Neukum <oneukum@suse.com> wrote:
>
> On 28.05.24 01:15, Dmitry Baryshkov wrote:
> > On Mon, May 27, 2024 at 02:26:36PM +0200, Oliver Neukum wrote:
> >> On 27.05.24 12:03, Dmitry Baryshkov wrote:
>
> Hi,
>
> >>> +struct yoga_c630_psy {
> >>> +   struct yoga_c630_ec *ec;
> >>> +   struct device *dev;
> >>> +   struct device_node *of_node;
> >>> +   struct notifier_block nb;
> >>> +   struct mutex lock;
> >>> +
> >>> +   struct power_supply *adp_psy;
> >>> +   struct power_supply *bat_psy;
> >>> +
> >>> +   unsigned long last_status_update;
> >>> +
> >>> +   bool adapter_online;
> >>> +
> >>> +   bool unit_mA;
> >>> +
> >>> +   unsigned int scale;
> >>
> >> why do you store unit_mA and scale? This looks redundant and like a source
> >> of confusion to me.
> >
> > Here we just followed the AML code in ACPI tables. The unit_mA is a
> > returned from the_BIX method, the 'scale' is used internally in the DSDT.
> > If you think that it's better, I can change all '* scale * 1000' to
> > 'if unit_mA then foo = bar * 10000 else foo = bar * 1000'.
>
> I think that would indeed be better. Implementation details of the DSDT
> should not dictate data structures in a kernel driver.

Ack.

>
>         Regards
>                 Oliver
>
Rob Herring May 28, 2024, 5:41 p.m. UTC | #3 
On Mon, 27 May 2024 13:03:45 +0300, Dmitry Baryshkov wrote:
> This adds binding, driver and the DT support for the Lenovo Yoga C630
> Embedded Controller, to provide battery information.
> 
> Support for this EC was implemented by Bjorn, who later could not work
> on this driver. I've picked this patchset up and updated it following
> the pending review comments.
> 
> DisplayPort support is still not a part of this patchset. It uses EC
> messages to provide AltMode information rather than implementing
> corresponding UCSI commands. However to have a cleaner uAPI story, the
> AltMode should be handled via the same Type-C port.
> 
> Merge strategy: the driver bits depend on the platform/arm64 patch,
> which adds interface for the subdrivers. I'd either ask to get that
> patch merged to the immutable branch, which then can be picked up by
> power/supply and USB trees or, to make life simpler, ack merging all
> driver bits e.g. through USB subsystem (I'm biased here since I plan to
> send more cleanups for the UCSI subsystem, which would otherwise result
> in cross-subsystem conflicts).
> 
> ---
> Changes in v3:
> - Split the driver into core and power supply drivers,
> - Added UCSI driver part, handling USB connections,
> - Fixed Bjorn's address in DT bindings (Brian Masney)
> - Changed power-role for both ports to be "dual" per UCSI
> - Link to v2: https://lore.kernel.org/linux-arm-msm/20230205152809.2233436-1-dmitry.baryshkov@linaro.org/
> 
> Changes in v2:
> - Dropped DP support for now, as the bindings are in process of being
>   discussed separately,
> - Merged dt patch into the same patchseries,
> - Removed the fixed serial number battery property,
> - Fixed indentation of dt bindings example,
> - Added property: reg and unevaluatedProperties to the connector
>   bindings.
> - Link to v1: https://lore.kernel.org/linux-arm-msm/20220810035424.2796777-1-bjorn.andersson@linaro.org/
> 
> ---
> Bjorn Andersson (2):
>       dt-bindings: power: supply: Add Lenovo Yoga C630 EC
>       arm64: dts: qcom: c630: Add Embedded Controller node
> 
> Dmitry Baryshkov (4):
>       platform: arm64: add Lenovo Yoga C630 WOS EC driver
>       usb: typec: ucsi: add Lenovo Yoga C630 glue driver
>       power: supply: lenovo_yoga_c630_battery: add Lenovo C630 driver
>       arm64: dts: qcom: sdm845: describe connections of USB/DP port
> 
>  .../bindings/power/supply/lenovo,yoga-c630-ec.yaml |  83 ++++
>  arch/arm64/boot/dts/qcom/sdm845.dtsi               |  53 ++-
>  .../boot/dts/qcom/sdm850-lenovo-yoga-c630.dts      |  76 ++++
>  drivers/platform/arm64/Kconfig                     |  14 +
>  drivers/platform/arm64/Makefile                    |   1 +
>  drivers/platform/arm64/lenovo-yoga-c630.c          | 279 ++++++++++++
>  drivers/power/supply/Kconfig                       |   9 +
>  drivers/power/supply/Makefile                      |   1 +
>  drivers/power/supply/lenovo_yoga_c630_battery.c    | 476 +++++++++++++++++++++
>  drivers/usb/typec/ucsi/Kconfig                     |   9 +
>  drivers/usb/typec/ucsi/Makefile                    |   1 +
>  drivers/usb/typec/ucsi/ucsi_yoga_c630.c            | 189 ++++++++
>  include/linux/platform_data/lenovo-yoga-c630.h     |  42 ++
>  13 files changed, 1232 insertions(+), 1 deletion(-)
> ---
> base-commit: 8314289a8d50a4e05d8ece1ae0445a3b57bb4d3b
> change-id: 20240527-yoga-ec-driver-76fd7f5ddae8
> 
> Best regards,
> --
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y qcom/sdm850-lenovo-yoga-c630.dtb' for 20240527-yoga-ec-driver-v3-0-327a9851dad5@linaro.org:

arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dtb: pinctrl@3400000: ec-int-state: 'oneOf' conditional failed, one must be fixed:
	'bias-disable', 'function', 'input-enable', 'pins' do not match any of the regexes: '-pins$', 'pinctrl-[0-9]+'
	False schema does not allow True
	from schema $id: http://devicetree.org/schemas/pinctrl/qcom,sdm845-pinctrl.yaml#