subject:"\[PATCH v4 1\/2\] power\: supply\: PCHG\: Peripheral device charger"

Re: [PATCH v4 1/2] power: supply: PCHG: Peripheral device charger

2021-04-05 Thread Sebastian Reichel

Hi,

On Sat, Jan 23, 2021 at 10:43:23AM -0800, Gwendal Grignou wrote:
> On Sat, Jan 23, 2021 at 7:53 AM Daisuke Nojiri  wrote:
> >
> > This patch adds a driver for PCHG (Peripheral CHarGer). PCHG is a
> > framework managing power supplies for peripheral devices.
> >
> > This driver creates a sysfs node for each peripheral charge port:
> >
> > /sys/class/power_supply/PCHGn
> >
> > where  is the index of a charge port.
> >
> > For example, when a stylus is connected to a NFC/WLC port, the node
> > prints:
> >
> > /sys/class/power_supply/PCHG0/
> > capacity=50
> > scope=Device
> > status=Charging
> > type=Battery
> >
> > Signed-off-by: Daisuke Nojiri 
> > ---
> > v1 -> v2
> > * Separate mfd/cros_ec_dev.c
> > * Make CONFIG_CHARGER_CROS_PCHG default to CONFIG_MFD_CROS_EC_DEV
> > v2 -> v3
> > * Return POWER_SUPPLY_SCOPE_DEVICE for POWER_SUPPLY_PROP_SCOPE
> > * POWER_SUPPLY_TYPE_WIRELESS -> POWER_SUPPLY_TYPE_BATTERY
> > v3 -> v4
> > * Return NOTIFY_DONE when a non-host event is notified.
> > ---
> >  drivers/power/supply/Kconfig  |  10 +
> >  drivers/power/supply/Makefile |   1 +
> >  .../power/supply/cros_peripheral_charger.c| 350 ++
> >  .../linux/platform_data/cros_ec_commands.h|  48 +++
> >  4 files changed, 409 insertions(+)
> >  create mode 100644 drivers/power/supply/cros_peripheral_charger.c
> >
> > diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> > index eec646c568b7be..407f9fbbc2bb50 100644
> > --- a/drivers/power/supply/Kconfig
> > +++ b/drivers/power/supply/Kconfig
> > @@ -714,6 +714,16 @@ config CHARGER_CROS_USBPD
> >   what is connected to USB PD ports from the EC and converts
> >   that into power_supply properties.
> >
> > +config CHARGER_CROS_PCHG
> > +   tristate "ChromeOS EC based peripheral charger"
> > +   depends on MFD_CROS_EC_DEV
> > +   default MFD_CROS_EC_DEV
> > +   help
> > + Say Y here to enable ChromeOS EC based peripheral charge driver.
> > + This driver gets various information about the devices connected 
> > to
> > + the peripheral charge ports from the EC and converts that into
> > + power_supply properties.
> > +
> >  config CHARGER_SC2731
> > tristate "Spreadtrum SC2731 charger driver"
> > depends on MFD_SC27XX_PMIC || COMPILE_TEST
> > diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> > index dd4b86318cd9bd..5263472a64809b 100644
> > --- a/drivers/power/supply/Makefile
> > +++ b/drivers/power/supply/Makefile
> > @@ -91,6 +91,7 @@ obj-$(CONFIG_CHARGER_TPS65217)+= 
> > tps65217_charger.o
> >  obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
> >  obj-$(CONFIG_AXP288_CHARGER)   += axp288_charger.o
> >  obj-$(CONFIG_CHARGER_CROS_USBPD)   += cros_usbpd-charger.o
> > +obj-$(CONFIG_CHARGER_CROS_PCHG)+= cros_peripheral_charger.o
> >  obj-$(CONFIG_CHARGER_SC2731)   += sc2731_charger.o
> >  obj-$(CONFIG_FUEL_GAUGE_SC27XX)+= sc27xx_fuel_gauge.o
> >  obj-$(CONFIG_CHARGER_UCS1002)  += ucs1002_power.o
> > diff --git a/drivers/power/supply/cros_peripheral_charger.c 
> > b/drivers/power/supply/cros_peripheral_charger.c
> > new file mode 100644
> > index 00..cd7db4bd5f90d0
> > --- /dev/null
> > +++ b/drivers/power/supply/cros_peripheral_charger.c
> > @@ -0,0 +1,350 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Power supply driver for ChromeOS EC based Peripheral Device Charger.
> > + *
> > + * Copyright 2020 Google LLC.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> Add
> 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define DRV_NAME   "cros-ec-pchg"
> > +#define PCHG_DIR_NAME  "PCHG%d"
> nit: to be coherent with function names, define CROS_EC_PCHG_DIR_NAME instead.
> Isn't PCHG0, PCHG1, ... too generic? Looking at other driver, we have
> strings like
> "max1721x-%012X", "wm831x-battery.%d",

Better name gives users better chance to understand what this is. I
think "peripheral%d" is already better than "PCHG%d", which is quite
cryptic. Does EC know the device type (e.g. stylus)?

> > +#define PCHG_DIR_NAME_LENGTH   sizeof("PCHG" 
> > __stringify(EC_PCHG_MAX_PORTS))
> > +#define PCHG_CACHE_UPDATE_DELAYmsecs_to_jiffies(500)
> > +
> > +struct port_data {
> > +   int port_number;
> > +   char name[PCHG_DIR_NAME_LENGTH];
> > +   struct power_supply *psy;
> > +   struct power_supply_desc psy_desc;
> > +   int psy_status;
> > +   int battery_percentage;
> > +   struct charger_data *charger;
> > +   unsigned long last_update;
> > +};
> > +
> > +struct charger_data {
> > +   struct device *dev;
> > +   struct cros_ec_dev *ec_dev;
> > +   struct cros_ec_device *ec_device;
> As far as I can see, ec_device is used only once, in

Re: [PATCH v4 1/2] power: supply: PCHG: Peripheral device charger

2021-01-23 Thread Gwendal Grignou

On Sat, Jan 23, 2021 at 7:53 AM Daisuke Nojiri  wrote:
>
> This patch adds a driver for PCHG (Peripheral CHarGer). PCHG is a
> framework managing power supplies for peripheral devices.
>
> This driver creates a sysfs node for each peripheral charge port:
>
> /sys/class/power_supply/PCHGn
>
> where  is the index of a charge port.
>
> For example, when a stylus is connected to a NFC/WLC port, the node
> prints:
>
> /sys/class/power_supply/PCHG0/
> capacity=50
> scope=Device
> status=Charging
> type=Battery
>
> Signed-off-by: Daisuke Nojiri 
> ---
> v1 -> v2
> * Separate mfd/cros_ec_dev.c
> * Make CONFIG_CHARGER_CROS_PCHG default to CONFIG_MFD_CROS_EC_DEV
> v2 -> v3
> * Return POWER_SUPPLY_SCOPE_DEVICE for POWER_SUPPLY_PROP_SCOPE
> * POWER_SUPPLY_TYPE_WIRELESS -> POWER_SUPPLY_TYPE_BATTERY
> v3 -> v4
> * Return NOTIFY_DONE when a non-host event is notified.
> ---
>  drivers/power/supply/Kconfig  |  10 +
>  drivers/power/supply/Makefile |   1 +
>  .../power/supply/cros_peripheral_charger.c| 350 ++
>  .../linux/platform_data/cros_ec_commands.h|  48 +++
>  4 files changed, 409 insertions(+)
>  create mode 100644 drivers/power/supply/cros_peripheral_charger.c
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index eec646c568b7be..407f9fbbc2bb50 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -714,6 +714,16 @@ config CHARGER_CROS_USBPD
>   what is connected to USB PD ports from the EC and converts
>   that into power_supply properties.
>
> +config CHARGER_CROS_PCHG
> +   tristate "ChromeOS EC based peripheral charger"
> +   depends on MFD_CROS_EC_DEV
> +   default MFD_CROS_EC_DEV
> +   help
> + Say Y here to enable ChromeOS EC based peripheral charge driver.
> + This driver gets various information about the devices connected to
> + the peripheral charge ports from the EC and converts that into
> + power_supply properties.
> +
>  config CHARGER_SC2731
> tristate "Spreadtrum SC2731 charger driver"
> depends on MFD_SC27XX_PMIC || COMPILE_TEST
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index dd4b86318cd9bd..5263472a64809b 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -91,6 +91,7 @@ obj-$(CONFIG_CHARGER_TPS65217)+= tps65217_charger.o
>  obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
>  obj-$(CONFIG_AXP288_CHARGER)   += axp288_charger.o
>  obj-$(CONFIG_CHARGER_CROS_USBPD)   += cros_usbpd-charger.o
> +obj-$(CONFIG_CHARGER_CROS_PCHG)+= cros_peripheral_charger.o
>  obj-$(CONFIG_CHARGER_SC2731)   += sc2731_charger.o
>  obj-$(CONFIG_FUEL_GAUGE_SC27XX)+= sc27xx_fuel_gauge.o
>  obj-$(CONFIG_CHARGER_UCS1002)  += ucs1002_power.o
> diff --git a/drivers/power/supply/cros_peripheral_charger.c 
> b/drivers/power/supply/cros_peripheral_charger.c
> new file mode 100644
> index 00..cd7db4bd5f90d0
> --- /dev/null
> +++ b/drivers/power/supply/cros_peripheral_charger.c
> @@ -0,0 +1,350 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Power supply driver for ChromeOS EC based Peripheral Device Charger.
> + *
> + * Copyright 2020 Google LLC.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
Add

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DRV_NAME   "cros-ec-pchg"
> +#define PCHG_DIR_NAME  "PCHG%d"
nit: to be coherent with function names, define CROS_EC_PCHG_DIR_NAME instead.
Isn't PCHG0, PCHG1, ... too generic? Looking at other driver, we have
strings like
"max1721x-%012X", "wm831x-battery.%d",
> +#define PCHG_DIR_NAME_LENGTH   sizeof("PCHG" __stringify(EC_PCHG_MAX_PORTS))
> +#define PCHG_CACHE_UPDATE_DELAYmsecs_to_jiffies(500)
> +
> +struct port_data {
> +   int port_number;
> +   char name[PCHG_DIR_NAME_LENGTH];
> +   struct power_supply *psy;
> +   struct power_supply_desc psy_desc;
> +   int psy_status;
> +   int battery_percentage;
> +   struct charger_data *charger;
> +   unsigned long last_update;
> +};
> +
> +struct charger_data {
> +   struct device *dev;
> +   struct cros_ec_dev *ec_dev;
> +   struct cros_ec_device *ec_device;
As far as I can see, ec_device is used only once, in cros_pchg_ec_command().
> +   int num_registered_psy;
> +   struct port_data *ports[EC_PCHG_MAX_PORTS];
> +   struct notifier_block notifier;
> +};
> +
> +static enum power_supply_property cros_pchg_props[] = {
> +   POWER_SUPPLY_PROP_STATUS,
> +   POWER_SUPPLY_PROP_CAPACITY,
> +   POWER_SUPPLY_PROP_SCOPE,
> +   /*
> +* todo: Add the following.
> +*
> +* POWER_SUPPLY_PROP_TECHNOLOGY,
> +* POWER_SUPPLY_PROP_ERROR,
> +* POWER_SUPPLY_PROP_SERIAL_NUMBER,
> +*

[PATCH v4 1/2] power: supply: PCHG: Peripheral device charger

2021-01-23 Thread Daisuke Nojiri

This patch adds a driver for PCHG (Peripheral CHarGer). PCHG is a
framework managing power supplies for peripheral devices.

This driver creates a sysfs node for each peripheral charge port:

/sys/class/power_supply/PCHGn

where  is the index of a charge port.

For example, when a stylus is connected to a NFC/WLC port, the node
prints:

/sys/class/power_supply/PCHG0/
capacity=50
scope=Device
status=Charging
type=Battery

Signed-off-by: Daisuke Nojiri 
---
v1 -> v2
* Separate mfd/cros_ec_dev.c
* Make CONFIG_CHARGER_CROS_PCHG default to CONFIG_MFD_CROS_EC_DEV
v2 -> v3
* Return POWER_SUPPLY_SCOPE_DEVICE for POWER_SUPPLY_PROP_SCOPE
* POWER_SUPPLY_TYPE_WIRELESS -> POWER_SUPPLY_TYPE_BATTERY
v3 -> v4
* Return NOTIFY_DONE when a non-host event is notified.
---
 drivers/power/supply/Kconfig  |  10 +
 drivers/power/supply/Makefile |   1 +
 .../power/supply/cros_peripheral_charger.c| 350 ++
 .../linux/platform_data/cros_ec_commands.h|  48 +++
 4 files changed, 409 insertions(+)
 create mode 100644 drivers/power/supply/cros_peripheral_charger.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index eec646c568b7be..407f9fbbc2bb50 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -714,6 +714,16 @@ config CHARGER_CROS_USBPD
  what is connected to USB PD ports from the EC and converts
  that into power_supply properties.
 
+config CHARGER_CROS_PCHG
+   tristate "ChromeOS EC based peripheral charger"
+   depends on MFD_CROS_EC_DEV
+   default MFD_CROS_EC_DEV
+   help
+ Say Y here to enable ChromeOS EC based peripheral charge driver.
+ This driver gets various information about the devices connected to
+ the peripheral charge ports from the EC and converts that into
+ power_supply properties.
+
 config CHARGER_SC2731
tristate "Spreadtrum SC2731 charger driver"
depends on MFD_SC27XX_PMIC || COMPILE_TEST
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index dd4b86318cd9bd..5263472a64809b 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -91,6 +91,7 @@ obj-$(CONFIG_CHARGER_TPS65217)+= tps65217_charger.o
 obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
 obj-$(CONFIG_AXP288_CHARGER)   += axp288_charger.o
 obj-$(CONFIG_CHARGER_CROS_USBPD)   += cros_usbpd-charger.o
+obj-$(CONFIG_CHARGER_CROS_PCHG)+= cros_peripheral_charger.o
 obj-$(CONFIG_CHARGER_SC2731)   += sc2731_charger.o
 obj-$(CONFIG_FUEL_GAUGE_SC27XX)+= sc27xx_fuel_gauge.o
 obj-$(CONFIG_CHARGER_UCS1002)  += ucs1002_power.o
diff --git a/drivers/power/supply/cros_peripheral_charger.c 
b/drivers/power/supply/cros_peripheral_charger.c
new file mode 100644
index 00..cd7db4bd5f90d0
--- /dev/null
+++ b/drivers/power/supply/cros_peripheral_charger.c
@@ -0,0 +1,350 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Power supply driver for ChromeOS EC based Peripheral Device Charger.
+ *
+ * Copyright 2020 Google LLC.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DRV_NAME   "cros-ec-pchg"
+#define PCHG_DIR_NAME  "PCHG%d"
+#define PCHG_DIR_NAME_LENGTH   sizeof("PCHG" __stringify(EC_PCHG_MAX_PORTS))
+#define PCHG_CACHE_UPDATE_DELAYmsecs_to_jiffies(500)
+
+struct port_data {
+   int port_number;
+   char name[PCHG_DIR_NAME_LENGTH];
+   struct power_supply *psy;
+   struct power_supply_desc psy_desc;
+   int psy_status;
+   int battery_percentage;
+   struct charger_data *charger;
+   unsigned long last_update;
+};
+
+struct charger_data {
+   struct device *dev;
+   struct cros_ec_dev *ec_dev;
+   struct cros_ec_device *ec_device;
+   int num_registered_psy;
+   struct port_data *ports[EC_PCHG_MAX_PORTS];
+   struct notifier_block notifier;
+};
+
+static enum power_supply_property cros_pchg_props[] = {
+   POWER_SUPPLY_PROP_STATUS,
+   POWER_SUPPLY_PROP_CAPACITY,
+   POWER_SUPPLY_PROP_SCOPE,
+   /*
+* todo: Add the following.
+*
+* POWER_SUPPLY_PROP_TECHNOLOGY,
+* POWER_SUPPLY_PROP_ERROR,
+* POWER_SUPPLY_PROP_SERIAL_NUMBER,
+*
+* POWER_SUPPLY_PROP_ONLINE can't be used because it indicates the
+* system is powered by AC.
+*/
+};
+
+static int cros_pchg_ec_command(const struct charger_data *charger,
+   unsigned int version,
+   unsigned int command,
+   const void *outdata,
+   unsigned int outsize,
+   void *indata,
+   unsigned int insize)
+{
+   struct cros_ec_dev *ec_dev = charger->ec_dev;
+   struct

Re: [PATCH v4 1/2] power: supply: PCHG: Peripheral device charger

Re: [PATCH v4 1/2] power: supply: PCHG: Peripheral device charger

[PATCH v4 1/2] power: supply: PCHG: Peripheral device charger

3 matches

Site Navigation

Mail list logo

Footer information