[PATCH 0/2] use correct values for startup time bits for AXP22X/AXP8XX PEKs

[PATCH 0/2] use correct values for startup time bits for AXP22X/AXP8XX PEKs

[Quentin Schulz]

According to their datasheets, the AXP221, AXP223, AXP288, AXP803,
AXP809 and AXP813 PEK have different values for startup time bits from
the AXP20X PEK (which are currently used for all the aforementioned PMICs).

This patch series adds support for platform_device_id to axp20x-pek driver
to support different startup time bits depending on PMIC's variant and set
the correct platform_device_id in the mfd.

This has been tested on AXP813. To perform the test, modify the value of
startup and/or shutdown of the pek in sysfs. Press the button soldered to
PEK input of the AXP the amount of time you set in sysfs. The PMIC shuts
down the board without needing to unplug any power supply. Press PEK button
the amount of time you set in sysfs for startup and the PMIC will start the
board. Note that the time bits are obivoulsy hardware reset to their
default when you unplug all power supplies from the board.

Thanks,
Quentin

Quentin Schulz (2):
  Input: axp20x-pek: add support for AXP221 PEK
  mfd: axp20x: use correct platform device id for many PEK

 drivers/input/misc/axp20x-pek.c | 62 ++++++++++++++++++++++++++++++++++-------
 drivers/mfd/axp20x.c            | 12 ++++----
 2 files changed, 58 insertions(+), 16 deletions(-)

-- 
2.11.0

[PATCH 1/2] Input: axp20x-pek: add support for AXP221 PEK

[Quentin Schulz]

The AXP221 has different values for startup time bits from the AXP20X.

This patch introduces a different platform_device_id to the driver and
adds the necessary code to handle the different platform_device_ids.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
 drivers/input/misc/axp20x-pek.c | 62 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 52 insertions(+), 10 deletions(-)

diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
index 38c79ebff033..3efa1de51569 100644
--- a/drivers/input/misc/axp20x-pek.c
+++ b/drivers/input/misc/axp20x-pek.c
@@ -32,6 +32,7 @@
 struct axp20x_pek {
 	struct axp20x_dev *axp20x;
 	struct input_dev *input;
+	struct attribute_group *attribute_group;
 	int irq_dbr;
 	int irq_dbf;
 };
@@ -41,14 +42,21 @@ struct axp20x_time {
 	unsigned int idx;
 };
 
-static const struct axp20x_time startup_time[] = {
+static const struct axp20x_time axp20x_startup_time[] = {
 	{ .time = 128,  .idx = 0 },
 	{ .time = 1000, .idx = 2 },
 	{ .time = 3000, .idx = 1 },
 	{ .time = 2000, .idx = 3 },
 };
 
-static const struct axp20x_time shutdown_time[] = {
+static const struct axp20x_time axp221_startup_time[] = {
+	{ .time = 128,  .idx = 0 },
+	{ .time = 1000, .idx = 1 },
+	{ .time = 2000, .idx = 2 },
+	{ .time = 3000, .idx = 3 },
+};
+
+static const struct axp20x_time axp20x_shutdown_time[] = {
 	{ .time = 4000,  .idx = 0 },
 	{ .time = 6000,  .idx = 1 },
 	{ .time = 8000,  .idx = 2 },
@@ -61,15 +69,20 @@ struct axp20x_pek_ext_attr {
 };
 
 static struct axp20x_pek_ext_attr axp20x_pek_startup_ext_attr = {
-	.p_time	= startup_time,
+	.p_time	= axp20x_startup_time,
 	.mask	= AXP20X_PEK_STARTUP_MASK,
 };
 
 static struct axp20x_pek_ext_attr axp20x_pek_shutdown_ext_attr = {
-	.p_time	= shutdown_time,
+	.p_time	= axp20x_shutdown_time,
 	.mask	= AXP20X_PEK_SHUTDOWN_MASK,
 };
 
+static struct axp20x_pek_ext_attr axp221_pek_startup_ext_attr = {
+	.p_time	= axp221_startup_time,
+	.mask	= AXP20X_PEK_STARTUP_MASK,
+};
+
 static struct axp20x_pek_ext_attr *get_axp_ext_attr(struct device_attribute *attr)
 {
 	return container_of(attr, struct dev_ext_attribute, attr)->var;
@@ -148,6 +161,11 @@ static struct dev_ext_attribute axp20x_dev_attr_startup = {
 	.var	= &axp20x_pek_startup_ext_attr,
 };
 
+static struct dev_ext_attribute axp221_dev_attr_startup = {
+	.attr	= __ATTR(startup, 0644, axp20x_show_ext_attr, axp20x_store_ext_attr),
+	.var	= &axp221_pek_startup_ext_attr,
+};
+
 static struct dev_ext_attribute axp20x_dev_attr_shutdown = {
 	.attr	= __ATTR(shutdown, 0644, axp20x_show_ext_attr, axp20x_store_ext_attr),
 	.var	= &axp20x_pek_shutdown_ext_attr,
@@ -159,10 +177,20 @@ static struct attribute *axp20x_attributes[] = {
 	NULL,
 };
 
+static struct attribute *axp221_attributes[] = {
+	&axp221_dev_attr_startup.attr.attr,
+	&axp20x_dev_attr_shutdown.attr.attr,
+	NULL,
+};
+
 static const struct attribute_group axp20x_attribute_group = {
 	.attrs = axp20x_attributes,
 };
 
+static const struct attribute_group axp221_attribute_group = {
+	.attrs = axp221_attributes,
+};
+
 static irqreturn_t axp20x_pek_irq(int irq, void *pwr)
 {
 	struct input_dev *idev = pwr;
@@ -184,9 +212,10 @@ static irqreturn_t axp20x_pek_irq(int irq, void *pwr)
 
 static void axp20x_remove_sysfs_group(void *_data)
 {
-	struct device *dev = _data;
+	struct platform_device *pdev = _data;
+	struct axp20x_pek *axp_pek = platform_get_drvdata(pdev);
 
-	sysfs_remove_group(&dev->kobj, &axp20x_attribute_group);
+	sysfs_remove_group(&pdev->dev.kobj, axp_pek->attribute_group);
 }
 
 static int axp20x_pek_probe_input_device(struct axp20x_pek *axp20x_pek,
@@ -313,17 +342,19 @@ static int axp20x_pek_probe(struct platform_device *pdev)
 			return error;
 	}
 
-	error = sysfs_create_group(&pdev->dev.kobj, &axp20x_attribute_group);
+	axp20x_pek->attribute_group = (struct attribute_group *)platform_get_device_id(pdev)->driver_data;
+
+	error = sysfs_create_group(&pdev->dev.kobj,
+				   axp20x_pek->attribute_group);
 	if (error) {
 		dev_err(&pdev->dev, "Failed to create sysfs attributes: %d\n",
 			error);
 		return error;
 	}
 
-	error = devm_add_action(&pdev->dev,
-				axp20x_remove_sysfs_group, &pdev->dev);
+	error = devm_add_action(&pdev->dev, axp20x_remove_sysfs_group, pdev);
 	if (error) {
-		axp20x_remove_sysfs_group(&pdev->dev);
+		axp20x_remove_sysfs_group(pdev);
 		dev_err(&pdev->dev, "Failed to add sysfs cleanup action: %d\n",
 			error);
 		return error;
@@ -358,8 +389,19 @@ static const struct dev_pm_ops axp20x_pek_pm_ops = {
 #endif
 };
 
+static const struct platform_device_id axp_pek_id_match[] = {
+	{
+		.name = "axp20x-pek",
+		.driver_data = (kernel_ulong_t)&axp20x_attribute_group,
+	}, {
+		.name = "axp221-pek",
+		.driver_data = (kernel_ulong_t)&axp221_attribute_group,
+	},
+};
+
 static struct platform_driver axp20x_pek_driver = {
 	.probe		= axp20x_pek_probe,
+	.id_table	= axp_pek_id_match,
 	.driver		= {
 		.name		= "axp20x-pek",
 		.pm		= &axp20x_pek_pm_ops,
-- 
2.11.0

[PATCH 2/2] mfd: axp20x: use correct platform device id for many PEK

[Quentin Schulz]

According to their datasheets, the AXP221, AXP223, AXP288, AXP803,
AXP809 and AXP813 PEK have different values for startup time bits from
the AXP20X, let's use the platform device id with the correct values.

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
 drivers/mfd/axp20x.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index ec4271ca065c..336de66ca408 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -677,7 +677,7 @@ static struct mfd_cell axp20x_cells[] = {
 
 static struct mfd_cell axp221_cells[] = {
 	{
-		.name		= "axp20x-pek",
+		.name		= "axp221-pek",
 		.num_resources	= ARRAY_SIZE(axp22x_pek_resources),
 		.resources	= axp22x_pek_resources,
 	}, {
@@ -702,7 +702,7 @@ static struct mfd_cell axp221_cells[] = {
 
 static struct mfd_cell axp223_cells[] = {
 	{
-		.name			= "axp20x-pek",
+		.name			= "axp221-pek",
 		.num_resources		= ARRAY_SIZE(axp22x_pek_resources),
 		.resources		= axp22x_pek_resources,
 	}, {
@@ -835,7 +835,7 @@ static struct mfd_cell axp288_cells[] = {
 		.resources = axp288_fuel_gauge_resources,
 	},
 	{
-		.name = "axp20x-pek",
+		.name = "axp221-pek",
 		.num_resources = ARRAY_SIZE(axp288_power_button_resources),
 		.resources = axp288_power_button_resources,
 	},
@@ -846,7 +846,7 @@ static struct mfd_cell axp288_cells[] = {
 
 static struct mfd_cell axp803_cells[] = {
 	{
-		.name			= "axp20x-pek",
+		.name			= "axp221-pek",
 		.num_resources		= ARRAY_SIZE(axp803_pek_resources),
 		.resources		= axp803_pek_resources,
 	},
@@ -862,7 +862,7 @@ static struct mfd_cell axp806_cells[] = {
 
 static struct mfd_cell axp809_cells[] = {
 	{
-		.name			= "axp20x-pek",
+		.name			= "axp221-pek",
 		.num_resources		= ARRAY_SIZE(axp809_pek_resources),
 		.resources		= axp809_pek_resources,
 	}, {
@@ -873,7 +873,7 @@ static struct mfd_cell axp809_cells[] = {
 
 static struct mfd_cell axp813_cells[] = {
 	{
-		.name			= "axp20x-pek",
+		.name			= "axp221-pek",
 		.num_resources		= ARRAY_SIZE(axp803_pek_resources),
 		.resources		= axp803_pek_resources,
 	}
-- 
2.11.0

Re: [PATCH 0/2] use correct values for startup time bits for AXP22X/AXP8XX PEKs

[Quentin Schulz]

Hi all,

On 17/07/2017 11:53, Quentin Schulz wrote:
> According to their datasheets, the AXP221, AXP223, AXP288, AXP803,
> AXP809 and AXP813 PEK have different values for startup time bits from
> the AXP20X PEK (which are currently used for all the aforementioned PMICs).
> 
> This patch series adds support for platform_device_id to axp20x-pek driver
> to support different startup time bits depending on PMIC's variant and set
> the correct platform_device_id in the mfd.
> 
> This has been tested on AXP813. To perform the test, modify the value of
> startup and/or shutdown of the pek in sysfs. Press the button soldered to
> PEK input of the AXP the amount of time you set in sysfs. The PMIC shuts
> down the board without needing to unplug any power supply. Press PEK button
> the amount of time you set in sysfs for startup and the PMIC will start the
> board. Note that the time bits are obivoulsy hardware reset to their
> default when you unplug all power supplies from the board.
> 

I forgot to add that it depends on this patch series (particularly the
third patch (3/6) "mfd: axp20x: Add support for AXP813 PMIC") from
Chen-Yu:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1428284.html

Quentin

> Thanks,
> Quentin
> 
> Quentin Schulz (2):
>   Input: axp20x-pek: add support for AXP221 PEK
>   mfd: axp20x: use correct platform device id for many PEK
> 
>  drivers/input/misc/axp20x-pek.c | 62 ++++++++++++++++++++++++++++++++++-------
>  drivers/mfd/axp20x.c            | 12 ++++----
>  2 files changed, 58 insertions(+), 16 deletions(-)
> 

-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 1/2] Input: axp20x-pek: add support for AXP221 PEK

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4837 bytes --]

Hi,

On Mon, Jul 17, 2017 at 11:53:06AM +0200, Quentin Schulz wrote:
> The AXP221 has different values for startup time bits from the AXP20X.
> 
> This patch introduces a different platform_device_id to the driver and
> adds the necessary code to handle the different platform_device_ids.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> ---
>  drivers/input/misc/axp20x-pek.c | 62 ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 52 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
> index 38c79ebff033..3efa1de51569 100644
> --- a/drivers/input/misc/axp20x-pek.c
> +++ b/drivers/input/misc/axp20x-pek.c
> @@ -32,6 +32,7 @@
>  struct axp20x_pek {
>  	struct axp20x_dev *axp20x;
>  	struct input_dev *input;
> +	struct attribute_group *attribute_group;
>  	int irq_dbr;
>  	int irq_dbf;
>  };
> @@ -41,14 +42,21 @@ struct axp20x_time {
>  	unsigned int idx;
>  };
>  
> -static const struct axp20x_time startup_time[] = {
> +static const struct axp20x_time axp20x_startup_time[] = {
>  	{ .time = 128,  .idx = 0 },
>  	{ .time = 1000, .idx = 2 },
>  	{ .time = 3000, .idx = 1 },
>  	{ .time = 2000, .idx = 3 },
>  };
>  
> -static const struct axp20x_time shutdown_time[] = {
> +static const struct axp20x_time axp221_startup_time[] = {
> +	{ .time = 128,  .idx = 0 },
> +	{ .time = 1000, .idx = 1 },
> +	{ .time = 2000, .idx = 2 },
> +	{ .time = 3000, .idx = 3 },
> +};
> +
> +static const struct axp20x_time axp20x_shutdown_time[] = {
>  	{ .time = 4000,  .idx = 0 },
>  	{ .time = 6000,  .idx = 1 },
>  	{ .time = 8000,  .idx = 2 },
> @@ -61,15 +69,20 @@ struct axp20x_pek_ext_attr {
>  };
>  
>  static struct axp20x_pek_ext_attr axp20x_pek_startup_ext_attr = {
> -	.p_time	= startup_time,
> +	.p_time	= axp20x_startup_time,
>  	.mask	= AXP20X_PEK_STARTUP_MASK,
>  };
>  
>  static struct axp20x_pek_ext_attr axp20x_pek_shutdown_ext_attr = {
> -	.p_time	= shutdown_time,
> +	.p_time	= axp20x_shutdown_time,
>  	.mask	= AXP20X_PEK_SHUTDOWN_MASK,
>  };
>  
> +static struct axp20x_pek_ext_attr axp221_pek_startup_ext_attr = {
> +	.p_time	= axp221_startup_time,
> +	.mask	= AXP20X_PEK_STARTUP_MASK,
> +};
> +
>  static struct axp20x_pek_ext_attr *get_axp_ext_attr(struct device_attribute *attr)
>  {
>  	return container_of(attr, struct dev_ext_attribute, attr)->var;
> @@ -148,6 +161,11 @@ static struct dev_ext_attribute axp20x_dev_attr_startup = {
>  	.var	= &axp20x_pek_startup_ext_attr,
>  };
>  
> +static struct dev_ext_attribute axp221_dev_attr_startup = {
> +	.attr	= __ATTR(startup, 0644, axp20x_show_ext_attr, axp20x_store_ext_attr),
> +	.var	= &axp221_pek_startup_ext_attr,
> +};
> +
>  static struct dev_ext_attribute axp20x_dev_attr_shutdown = {
>  	.attr	= __ATTR(shutdown, 0644, axp20x_show_ext_attr, axp20x_store_ext_attr),
>  	.var	= &axp20x_pek_shutdown_ext_attr,
> @@ -159,10 +177,20 @@ static struct attribute *axp20x_attributes[] = {
>  	NULL,
>  };
>  
> +static struct attribute *axp221_attributes[] = {
> +	&axp221_dev_attr_startup.attr.attr,
> +	&axp20x_dev_attr_shutdown.attr.attr,
> +	NULL,
> +};
> +
>  static const struct attribute_group axp20x_attribute_group = {
>  	.attrs = axp20x_attributes,
>  };
>  
> +static const struct attribute_group axp221_attribute_group = {
> +	.attrs = axp221_attributes,
> +};
> +
>  static irqreturn_t axp20x_pek_irq(int irq, void *pwr)
>  {
>  	struct input_dev *idev = pwr;
> @@ -184,9 +212,10 @@ static irqreturn_t axp20x_pek_irq(int irq, void *pwr)
>  
>  static void axp20x_remove_sysfs_group(void *_data)
>  {
> -	struct device *dev = _data;
> +	struct platform_device *pdev = _data;
> +	struct axp20x_pek *axp_pek = platform_get_drvdata(pdev);
>  
> -	sysfs_remove_group(&dev->kobj, &axp20x_attribute_group);
> +	sysfs_remove_group(&pdev->dev.kobj, axp_pek->attribute_group);
>  }
>  
>  static int axp20x_pek_probe_input_device(struct axp20x_pek *axp20x_pek,
> @@ -313,17 +342,19 @@ static int axp20x_pek_probe(struct platform_device *pdev)
>  			return error;
>  	}
>  
> -	error = sysfs_create_group(&pdev->dev.kobj, &axp20x_attribute_group);
> +	axp20x_pek->attribute_group = (struct attribute_group *)platform_get_device_id(pdev)->driver_data;

That line is too long, and you don't check the returned pointer of
platform_get_device_id. You should split it and check for the pointer.

> +
> +	error = sysfs_create_group(&pdev->dev.kobj,
> +				   axp20x_pek->attribute_group);

Wouldn't it make more sense to just store the startup_time structure
in the axp20x_pek structure, rather than duplicating all this?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 0/2] use correct values for startup time bits for AXP22X/AXP8XX PEKs

[Lee Jones]

On Mon, 17 Jul 2017, Quentin Schulz wrote:

> Hi all,
> 
> On 17/07/2017 11:53, Quentin Schulz wrote:
> > According to their datasheets, the AXP221, AXP223, AXP288, AXP803,
> > AXP809 and AXP813 PEK have different values for startup time bits from
> > the AXP20X PEK (which are currently used for all the aforementioned PMICs).
> > 
> > This patch series adds support for platform_device_id to axp20x-pek driver
> > to support different startup time bits depending on PMIC's variant and set
> > the correct platform_device_id in the mfd.
> > 
> > This has been tested on AXP813. To perform the test, modify the value of
> > startup and/or shutdown of the pek in sysfs. Press the button soldered to
> > PEK input of the AXP the amount of time you set in sysfs. The PMIC shuts
> > down the board without needing to unplug any power supply. Press PEK button
> > the amount of time you set in sysfs for startup and the PMIC will start the
> > board. Note that the time bits are obivoulsy hardware reset to their
> > default when you unplug all power supplies from the board.
> > 
> 
> I forgot to add that it depends on this patch series (particularly the
> third patch (3/6) "mfd: axp20x: Add support for AXP813 PMIC") from
> Chen-Yu:
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1428284.html

So you are going to re-send?

> > Quentin Schulz (2):
> >   Input: axp20x-pek: add support for AXP221 PEK
> >   mfd: axp20x: use correct platform device id for many PEK
> > 
> >  drivers/input/misc/axp20x-pek.c | 62 ++++++++++++++++++++++++++++++++++-------
> >  drivers/mfd/axp20x.c            | 12 ++++----
> >  2 files changed, 58 insertions(+), 16 deletions(-)
> > 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/2] mfd: axp20x: use correct platform device id for many PEK

[Lee Jones]

On Mon, 17 Jul 2017, Quentin Schulz wrote:

> According to their datasheets, the AXP221, AXP223, AXP288, AXP803,
> AXP809 and AXP813 PEK have different values for startup time bits from
> the AXP20X, let's use the platform device id with the correct values.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> ---
>  drivers/mfd/axp20x.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Patch is find, but are these names reference from platform data
anywhere i.e. are we going to break anything by applying it?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 0/2] use correct values for startup time bits for AXP22X/AXP8XX PEKs

[Quentin Schulz]

Hi Lee,

On 18/07/2017 09:18, Lee Jones wrote:
> On Mon, 17 Jul 2017, Quentin Schulz wrote:
> 
>> Hi all,
>>
>> On 17/07/2017 11:53, Quentin Schulz wrote:
>>> According to their datasheets, the AXP221, AXP223, AXP288, AXP803,
>>> AXP809 and AXP813 PEK have different values for startup time bits from
>>> the AXP20X PEK (which are currently used for all the aforementioned PMICs).
>>>
>>> This patch series adds support for platform_device_id to axp20x-pek driver
>>> to support different startup time bits depending on PMIC's variant and set
>>> the correct platform_device_id in the mfd.
>>>
>>> This has been tested on AXP813. To perform the test, modify the value of
>>> startup and/or shutdown of the pek in sysfs. Press the button soldered to
>>> PEK input of the AXP the amount of time you set in sysfs. The PMIC shuts
>>> down the board without needing to unplug any power supply. Press PEK button
>>> the amount of time you set in sysfs for startup and the PMIC will start the
>>> board. Note that the time bits are obivoulsy hardware reset to their
>>> default when you unplug all power supplies from the board.
>>>
>>
>> I forgot to add that it depends on this patch series (particularly the
>> third patch (3/6) "mfd: axp20x: Add support for AXP813 PMIC") from
>> Chen-Yu:
>> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1428284.html
> 
> So you are going to re-send?
> 

You put your acked-by just yesterday on this patch. Do I need to resend
it anyway?

The link I provided seems to be really slow/not working today, so I give
you another one for reference: https://patchwork.kernel.org/patch/9803439/

Thanks,
Quentin

>>> Quentin Schulz (2):
>>>   Input: axp20x-pek: add support for AXP221 PEK
>>>   mfd: axp20x: use correct platform device id for many PEK
>>>
>>>  drivers/input/misc/axp20x-pek.c | 62 ++++++++++++++++++++++++++++++++++-------
>>>  drivers/mfd/axp20x.c            | 12 ++++----
>>>  2 files changed, 58 insertions(+), 16 deletions(-)
>>>
>>
> 

-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 1/2] Input: axp20x-pek: add support for AXP221 PEK

[Quentin Schulz]

[-- Attachment #1.1: Type: text/plain, Size: 5342 bytes --]

Hi Maxime,

On 17/07/2017 13:29, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Jul 17, 2017 at 11:53:06AM +0200, Quentin Schulz wrote:
>> The AXP221 has different values for startup time bits from the AXP20X.
>>
>> This patch introduces a different platform_device_id to the driver and
>> adds the necessary code to handle the different platform_device_ids.
>>
>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
>> ---
>>  drivers/input/misc/axp20x-pek.c | 62 ++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 52 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
>> index 38c79ebff033..3efa1de51569 100644
>> --- a/drivers/input/misc/axp20x-pek.c
>> +++ b/drivers/input/misc/axp20x-pek.c
>> @@ -32,6 +32,7 @@
>>  struct axp20x_pek {
>>  	struct axp20x_dev *axp20x;
>>  	struct input_dev *input;
>> +	struct attribute_group *attribute_group;
>>  	int irq_dbr;
>>  	int irq_dbf;
>>  };
>> @@ -41,14 +42,21 @@ struct axp20x_time {
>>  	unsigned int idx;
>>  };
>>  
>> -static const struct axp20x_time startup_time[] = {
>> +static const struct axp20x_time axp20x_startup_time[] = {
>>  	{ .time = 128,  .idx = 0 },
>>  	{ .time = 1000, .idx = 2 },
>>  	{ .time = 3000, .idx = 1 },
>>  	{ .time = 2000, .idx = 3 },
>>  };
>>  
>> -static const struct axp20x_time shutdown_time[] = {
>> +static const struct axp20x_time axp221_startup_time[] = {
>> +	{ .time = 128,  .idx = 0 },
>> +	{ .time = 1000, .idx = 1 },
>> +	{ .time = 2000, .idx = 2 },
>> +	{ .time = 3000, .idx = 3 },
>> +};
>> +
>> +static const struct axp20x_time axp20x_shutdown_time[] = {
>>  	{ .time = 4000,  .idx = 0 },
>>  	{ .time = 6000,  .idx = 1 },
>>  	{ .time = 8000,  .idx = 2 },
>> @@ -61,15 +69,20 @@ struct axp20x_pek_ext_attr {
>>  };
>>  
>>  static struct axp20x_pek_ext_attr axp20x_pek_startup_ext_attr = {
>> -	.p_time	= startup_time,
>> +	.p_time	= axp20x_startup_time,
>>  	.mask	= AXP20X_PEK_STARTUP_MASK,
>>  };
>>  
>>  static struct axp20x_pek_ext_attr axp20x_pek_shutdown_ext_attr = {
>> -	.p_time	= shutdown_time,
>> +	.p_time	= axp20x_shutdown_time,
>>  	.mask	= AXP20X_PEK_SHUTDOWN_MASK,
>>  };
>>  
>> +static struct axp20x_pek_ext_attr axp221_pek_startup_ext_attr = {
>> +	.p_time	= axp221_startup_time,
>> +	.mask	= AXP20X_PEK_STARTUP_MASK,
>> +};
>> +
>>  static struct axp20x_pek_ext_attr *get_axp_ext_attr(struct device_attribute *attr)
>>  {
>>  	return container_of(attr, struct dev_ext_attribute, attr)->var;
>> @@ -148,6 +161,11 @@ static struct dev_ext_attribute axp20x_dev_attr_startup = {
>>  	.var	= &axp20x_pek_startup_ext_attr,
>>  };
>>  
>> +static struct dev_ext_attribute axp221_dev_attr_startup = {
>> +	.attr	= __ATTR(startup, 0644, axp20x_show_ext_attr, axp20x_store_ext_attr),
>> +	.var	= &axp221_pek_startup_ext_attr,
>> +};
>> +
>>  static struct dev_ext_attribute axp20x_dev_attr_shutdown = {
>>  	.attr	= __ATTR(shutdown, 0644, axp20x_show_ext_attr, axp20x_store_ext_attr),
>>  	.var	= &axp20x_pek_shutdown_ext_attr,
>> @@ -159,10 +177,20 @@ static struct attribute *axp20x_attributes[] = {
>>  	NULL,
>>  };
>>  
>> +static struct attribute *axp221_attributes[] = {
>> +	&axp221_dev_attr_startup.attr.attr,
>> +	&axp20x_dev_attr_shutdown.attr.attr,
>> +	NULL,
>> +};
>> +
>>  static const struct attribute_group axp20x_attribute_group = {
>>  	.attrs = axp20x_attributes,
>>  };
>>  
>> +static const struct attribute_group axp221_attribute_group = {
>> +	.attrs = axp221_attributes,
>> +};
>> +
>>  static irqreturn_t axp20x_pek_irq(int irq, void *pwr)
>>  {
>>  	struct input_dev *idev = pwr;
>> @@ -184,9 +212,10 @@ static irqreturn_t axp20x_pek_irq(int irq, void *pwr)
>>  
>>  static void axp20x_remove_sysfs_group(void *_data)
>>  {
>> -	struct device *dev = _data;
>> +	struct platform_device *pdev = _data;
>> +	struct axp20x_pek *axp_pek = platform_get_drvdata(pdev);
>>  
>> -	sysfs_remove_group(&dev->kobj, &axp20x_attribute_group);
>> +	sysfs_remove_group(&pdev->dev.kobj, axp_pek->attribute_group);
>>  }
>>  
>>  static int axp20x_pek_probe_input_device(struct axp20x_pek *axp20x_pek,
>> @@ -313,17 +342,19 @@ static int axp20x_pek_probe(struct platform_device *pdev)
>>  			return error;
>>  	}
>>  
>> -	error = sysfs_create_group(&pdev->dev.kobj, &axp20x_attribute_group);
>> +	axp20x_pek->attribute_group = (struct attribute_group *)platform_get_device_id(pdev)->driver_data;
> 
> That line is too long, and you don't check the returned pointer of
> platform_get_device_id. You should split it and check for the pointer.
> 

That's what I've done in a previous driver:
http://elixir.free-electrons.com/linux/latest/source/drivers/iio/adc/axp20x_adc.c#L543

I'll do what you said anyway.

>> +
>> +	error = sysfs_create_group(&pdev->dev.kobj,
>> +				   axp20x_pek->attribute_group);
> 
> Wouldn't it make more sense to just store the startup_time structure
> in the axp20x_pek structure, rather than duplicating all this?
> 

I don't know. Basically, you would need to recreate manually all the
structures in the probe function, but why not.

Thanks,
Quentin

-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 0/2] use correct values for startup time bits for AXP22X/AXP8XX PEKs

[Chen-Yu Tsai]

On Tue, Jul 18, 2017 at 3:25 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> Hi Lee,
>
> On 18/07/2017 09:18, Lee Jones wrote:
>> On Mon, 17 Jul 2017, Quentin Schulz wrote:
>>
>>> Hi all,
>>>
>>> On 17/07/2017 11:53, Quentin Schulz wrote:
>>>> According to their datasheets, the AXP221, AXP223, AXP288, AXP803,
>>>> AXP809 and AXP813 PEK have different values for startup time bits from
>>>> the AXP20X PEK (which are currently used for all the aforementioned PMICs).
>>>>
>>>> This patch series adds support for platform_device_id to axp20x-pek driver
>>>> to support different startup time bits depending on PMIC's variant and set
>>>> the correct platform_device_id in the mfd.
>>>>
>>>> This has been tested on AXP813. To perform the test, modify the value of
>>>> startup and/or shutdown of the pek in sysfs. Press the button soldered to
>>>> PEK input of the AXP the amount of time you set in sysfs. The PMIC shuts
>>>> down the board without needing to unplug any power supply. Press PEK button
>>>> the amount of time you set in sysfs for startup and the PMIC will start the
>>>> board. Note that the time bits are obivoulsy hardware reset to their
>>>> default when you unplug all power supplies from the board.
>>>>
>>>
>>> I forgot to add that it depends on this patch series (particularly the
>>> third patch (3/6) "mfd: axp20x: Add support for AXP813 PMIC") from
>>> Chen-Yu:
>>> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1428284.html
>>
>> So you are going to re-send?
>>
>
> You put your acked-by just yesterday on this patch. Do I need to resend
> it anyway?
>
> The link I provided seems to be really slow/not working today, so I give
> you another one for reference: https://patchwork.kernel.org/patch/9803439/

Or rather, would you like me to merge the AXP813 changes into my next
version, and you resend the rest for faster separate inclusion?

ChenYu

> Thanks,
> Quentin
>
>>>> Quentin Schulz (2):
>>>>   Input: axp20x-pek: add support for AXP221 PEK
>>>>   mfd: axp20x: use correct platform device id for many PEK
>>>>
>>>>  drivers/input/misc/axp20x-pek.c | 62 ++++++++++++++++++++++++++++++++++-------
>>>>  drivers/mfd/axp20x.c            | 12 ++++----
>>>>  2 files changed, 58 insertions(+), 16 deletions(-)
>>>>
>>>
>>
>
> --
> Quentin Schulz, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

Re: [PATCH 2/2] mfd: axp20x: use correct platform device id for many PEK

[Quentin Schulz]

Hi Lee,

On 18/07/2017 09:19, Lee Jones wrote:
> On Mon, 17 Jul 2017, Quentin Schulz wrote:
> 
>> According to their datasheets, the AXP221, AXP223, AXP288, AXP803,
>> AXP809 and AXP813 PEK have different values for startup time bits from
>> the AXP20X, let's use the platform device id with the correct values.
>>
>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
>> ---
>>  drivers/mfd/axp20x.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> Patch is find, but are these names reference from platform data
> anywhere i.e. are we going to break anything by applying it?
> 

I don't really understand what you're asking.

We need the first patch of this patch series to be applied before the
second patch or axp20x-pek driver wouldn't be probed anymore.

There is no Device Tree declaring axp20x-pek and there is no support for
Device Tree probing in the driver.

I don't see how I could break anything with these patches. Could you
explain with an abstract example, please? I might not break anything
here but it's better to know now what I could have broken in another
situation/with another patch series so I won't make that mistake in the
future.

Oh, but this patch series would change the name of the directory exposed
in sysfs (/sys/bus/platform/devices/axp221-pek/*). Is that what you were
afraid of?

Thanks,
Quentin
-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 1/2] Input: axp20x-pek: add support for AXP221 PEK

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 733 bytes --]

On Tue, Jul 18, 2017 at 09:36:04AM +0200, Quentin Schulz wrote:
> >> +
> >> +	error = sysfs_create_group(&pdev->dev.kobj,
> >> +				   axp20x_pek->attribute_group);
> > 
> > Wouldn't it make more sense to just store the startup_time structure
> > in the axp20x_pek structure, rather than duplicating all this?
> > 
> 
> I don't know. Basically, you would need to recreate manually all the
> structures in the probe function, but why not.

Why?

You have access to axp20x_pek in the readout function. You can just
use the table stored in the structure here, and just ignore any
argument you might have.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 1/2] Input: axp20x-pek: add support for AXP221 PEK

[Quentin Schulz]

[-- Attachment #1.1: Type: text/plain, Size: 1263 bytes --]

Hi Maxime,

On 18/07/2017 09:57, Maxime Ripard wrote:
> On Tue, Jul 18, 2017 at 09:36:04AM +0200, Quentin Schulz wrote:
>>>> +
>>>> +	error = sysfs_create_group(&pdev->dev.kobj,
>>>> +				   axp20x_pek->attribute_group);
>>>
>>> Wouldn't it make more sense to just store the startup_time structure
>>> in the axp20x_pek structure, rather than duplicating all this?
>>>
>>
>> I don't know. Basically, you would need to recreate manually all the
>> structures in the probe function, but why not.
> 
> Why?
> 
> You have access to axp20x_pek in the readout function. You can just
> use the table stored in the structure here, and just ignore any
> argument you might have.
> 

All I'm saying is that:
struct axp20x_time is referenced in struct axp20x_pek_ext_attr which in
turn is referenced in struct dev_ext_attribute which is referenced in
struct attribute which is reference in const struct attribute_group
which is finally used to add/remove a sysfs group programmatically.

The earlier in the chain you decide to stop duplicating code, the most
structures you have to programmatically create. That was my point.

Quentin

-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 2/2] mfd: axp20x: use correct platform device id for many PEK

[Lee Jones]

On Tue, 18 Jul 2017, Quentin Schulz wrote:

> Hi Lee,
> 
> On 18/07/2017 09:19, Lee Jones wrote:
> > On Mon, 17 Jul 2017, Quentin Schulz wrote:
> > 
> >> According to their datasheets, the AXP221, AXP223, AXP288, AXP803,
> >> AXP809 and AXP813 PEK have different values for startup time bits from
> >> the AXP20X, let's use the platform device id with the correct values.
> >>
> >> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> >> ---
> >>  drivers/mfd/axp20x.c | 12 ++++++------
> >>  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > Patch is find, but are these names reference from platform data
> > anywhere i.e. are we going to break anything by applying it?
> > 
> 
> I don't really understand what you're asking.

Yes, I guess that was a little Fringlish, apologies for my haste.

> We need the first patch of this patch series to be applied before the
> second patch or axp20x-pek driver wouldn't be probed anymore.
> 
> There is no Device Tree declaring axp20x-pek and there is no support for
> Device Tree probing in the driver.
> 
> I don't see how I could break anything with these patches. Could you
> explain with an abstract example, please? I might not break anything
> here but it's better to know now what I could have broken in another
> situation/with another patch series so I won't make that mistake in the
> future.
> 
> Oh, but this patch series would change the name of the directory exposed
> in sysfs (/sys/bus/platform/devices/axp221-pek/*). Is that what you were
> afraid of?

I'm worried about any breakage in terms of name referencing.

If this driver is DT only, then the concern is less, but in the olden
days, we used to conduct device/driver binding using the name.  Ergo,
if you change the name in the driver without updating the device
registration, we would not bind and .probe() would not be called.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 0/2] use correct values for startup time bits for AXP22X/AXP8XX PEKs

[Lee Jones]

On Tue, 18 Jul 2017, Chen-Yu Tsai wrote:

> On Tue, Jul 18, 2017 at 3:25 PM, Quentin Schulz
> <quentin.schulz@free-electrons.com> wrote:
> > Hi Lee,
> >
> > On 18/07/2017 09:18, Lee Jones wrote:
> >> On Mon, 17 Jul 2017, Quentin Schulz wrote:
> >>
> >>> Hi all,
> >>>
> >>> On 17/07/2017 11:53, Quentin Schulz wrote:
> >>>> According to their datasheets, the AXP221, AXP223, AXP288, AXP803,
> >>>> AXP809 and AXP813 PEK have different values for startup time bits from
> >>>> the AXP20X PEK (which are currently used for all the aforementioned PMICs).
> >>>>
> >>>> This patch series adds support for platform_device_id to axp20x-pek driver
> >>>> to support different startup time bits depending on PMIC's variant and set
> >>>> the correct platform_device_id in the mfd.
> >>>>
> >>>> This has been tested on AXP813. To perform the test, modify the value of
> >>>> startup and/or shutdown of the pek in sysfs. Press the button soldered to
> >>>> PEK input of the AXP the amount of time you set in sysfs. The PMIC shuts
> >>>> down the board without needing to unplug any power supply. Press PEK button
> >>>> the amount of time you set in sysfs for startup and the PMIC will start the
> >>>> board. Note that the time bits are obivoulsy hardware reset to their
> >>>> default when you unplug all power supplies from the board.
> >>>>
> >>>
> >>> I forgot to add that it depends on this patch series (particularly the
> >>> third patch (3/6) "mfd: axp20x: Add support for AXP813 PMIC") from
> >>> Chen-Yu:
> >>> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1428284.html
> >>
> >> So you are going to re-send?
> >>
> >
> > You put your acked-by just yesterday on this patch. Do I need to resend
> > it anyway?
> >
> > The link I provided seems to be really slow/not working today, so I give
> > you another one for reference: https://patchwork.kernel.org/patch/9803439/
> 
> Or rather, would you like me to merge the AXP813 changes into my next
> version, and you resend the rest for faster separate inclusion?

What I'd really like is for one of you to collect up the patches which
depend on each other (in the correct order of course), along with the
Acks you've obtained and send as a single set.  That way, I am less
likely to mess up the ordering when applying.

> >>>> Quentin Schulz (2):
> >>>>   Input: axp20x-pek: add support for AXP221 PEK
> >>>>   mfd: axp20x: use correct platform device id for many PEK
> >>>>
> >>>>  drivers/input/misc/axp20x-pek.c | 62 ++++++++++++++++++++++++++++++++++-------
> >>>>  drivers/mfd/axp20x.c            | 12 ++++----
> >>>>  2 files changed, 58 insertions(+), 16 deletions(-)
> >>>>
> >>>
> >>
> >

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/2] mfd: axp20x: use correct platform device id for many PEK

[Quentin Schulz]

Hi Lee,

On 18/07/2017 11:49, Lee Jones wrote:
> On Tue, 18 Jul 2017, Quentin Schulz wrote:
> 
>> Hi Lee,
>>
>> On 18/07/2017 09:19, Lee Jones wrote:
>>> On Mon, 17 Jul 2017, Quentin Schulz wrote:
>>>
>>>> According to their datasheets, the AXP221, AXP223, AXP288, AXP803,
>>>> AXP809 and AXP813 PEK have different values for startup time bits from
>>>> the AXP20X, let's use the platform device id with the correct values.
>>>>
>>>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
>>>> ---
>>>>  drivers/mfd/axp20x.c | 12 ++++++------
>>>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> Patch is find, but are these names reference from platform data
>>> anywhere i.e. are we going to break anything by applying it?
>>>
>>
>> I don't really understand what you're asking.
> 
> Yes, I guess that was a little Fringlish, apologies for my haste.
> 
>> We need the first patch of this patch series to be applied before the
>> second patch or axp20x-pek driver wouldn't be probed anymore.
>>
>> There is no Device Tree declaring axp20x-pek and there is no support for
>> Device Tree probing in the driver.
>>
>> I don't see how I could break anything with these patches. Could you
>> explain with an abstract example, please? I might not break anything
>> here but it's better to know now what I could have broken in another
>> situation/with another patch series so I won't make that mistake in the
>> future.
>>
>> Oh, but this patch series would change the name of the directory exposed
>> in sysfs (/sys/bus/platform/devices/axp221-pek/*). Is that what you were
>> afraid of?
> 
> I'm worried about any breakage in terms of name referencing.
> 
> If this driver is DT only, then the concern is less, but in the olden
> days, we used to conduct device/driver binding using the name.  Ergo,
> if you change the name in the driver without updating the device
> registration, we would not bind and .probe() would not be called.
> 

Previous to this patch set, the axp20x-pek driver would have no
platform_device_id table set. The name attribute in the
platform_driver.driver was (and still is): "axp20x-pek". As I
understand, the MFD subsystem would use the name of the driver to make
the connection between the name defined in the mfd cell and the one in
the driver.

Now I've a platform device id table that, if I understood correctly,
would be used by the MFD subsystem to make the connection between the
name defined in the mfd cell and the platform device id table.

My platform device id table is as following:

static const struct platform_device_id axp_pek_id_match[] = {

        {

                .name = "axp20x-pek",

                .driver_data = (kernel_ulong_t)&axp20x_attribute_group,

        }, {

                .name = "axp221-pek",

                .driver_data = (kernel_ulong_t)&axp221_attribute_group,

        },

};


Thus, by keeping axp20x-pek as one of the platform device id, we do not
break anything since everything that makes the connection with the
driver name would also make the connection with the platform device id.
Right?

Basically without this patch, axp20x-pek still probes (I've just tested
to make sure), with "axp20x-pek" platform device id, as it does today
with except with "axp20x-pek" driver name.

Does it make sense? Do I answer your worries?

Thanks,
Quentin

-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 2/2] mfd: axp20x: use correct platform device id for many PEK

[Lee Jones]

On Tue, 18 Jul 2017, Quentin Schulz wrote:

> Hi Lee,
> 
> On 18/07/2017 11:49, Lee Jones wrote:
> > On Tue, 18 Jul 2017, Quentin Schulz wrote:
> > 
> >> Hi Lee,
> >>
> >> On 18/07/2017 09:19, Lee Jones wrote:
> >>> On Mon, 17 Jul 2017, Quentin Schulz wrote:
> >>>
> >>>> According to their datasheets, the AXP221, AXP223, AXP288, AXP803,
> >>>> AXP809 and AXP813 PEK have different values for startup time bits from
> >>>> the AXP20X, let's use the platform device id with the correct values.
> >>>>
> >>>> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> >>>> ---
> >>>>  drivers/mfd/axp20x.c | 12 ++++++------
> >>>>  1 file changed, 6 insertions(+), 6 deletions(-)
> >>>
> >>> Patch is find, but are these names reference from platform data
> >>> anywhere i.e. are we going to break anything by applying it?
> >>>
> >>
> >> I don't really understand what you're asking.
> > 
> > Yes, I guess that was a little Fringlish, apologies for my haste.
> > 
> >> We need the first patch of this patch series to be applied before the
> >> second patch or axp20x-pek driver wouldn't be probed anymore.
> >>
> >> There is no Device Tree declaring axp20x-pek and there is no support for
> >> Device Tree probing in the driver.
> >>
> >> I don't see how I could break anything with these patches. Could you
> >> explain with an abstract example, please? I might not break anything
> >> here but it's better to know now what I could have broken in another
> >> situation/with another patch series so I won't make that mistake in the
> >> future.
> >>
> >> Oh, but this patch series would change the name of the directory exposed
> >> in sysfs (/sys/bus/platform/devices/axp221-pek/*). Is that what you were
> >> afraid of?
> > 
> > I'm worried about any breakage in terms of name referencing.
> > 
> > If this driver is DT only, then the concern is less, but in the olden
> > days, we used to conduct device/driver binding using the name.  Ergo,
> > if you change the name in the driver without updating the device
> > registration, we would not bind and .probe() would not be called.
> > 
> 
> Previous to this patch set, the axp20x-pek driver would have no
> platform_device_id table set. The name attribute in the
> platform_driver.driver was (and still is): "axp20x-pek". As I
> understand, the MFD subsystem would use the name of the driver to make
> the connection between the name defined in the mfd cell and the one in
> the driver.
> 
> Now I've a platform device id table that, if I understood correctly,
> would be used by the MFD subsystem to make the connection between the
> name defined in the mfd cell and the platform device id table.
> 
> My platform device id table is as following:
> 
> static const struct platform_device_id axp_pek_id_match[] = {
> 
>         {
> 
>                 .name = "axp20x-pek",
> 
>                 .driver_data = (kernel_ulong_t)&axp20x_attribute_group,
> 
>         }, {
> 
>                 .name = "axp221-pek",
> 
>                 .driver_data = (kernel_ulong_t)&axp221_attribute_group,
> 
>         },
> 
> };
> 
> 
> Thus, by keeping axp20x-pek as one of the platform device id, we do not
> break anything since everything that makes the connection with the
> driver name would also make the connection with the platform device id.
> Right?
> 
> Basically without this patch, axp20x-pek still probes (I've just tested
> to make sure), with "axp20x-pek" platform device id, as it does today
> with except with "axp20x-pek" driver name.
> 
> Does it make sense? Do I answer your worries?

Yes, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/2] mfd: axp20x: use correct platform device id for many PEK

[Lee Jones]

On Mon, 17 Jul 2017, Quentin Schulz wrote:

> According to their datasheets, the AXP221, AXP223, AXP288, AXP803,
> AXP809 and AXP813 PEK have different values for startup time bits from
> the AXP20X, let's use the platform device id with the correct values.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> ---
>  drivers/mfd/axp20x.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
  
-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 0/2] use correct values for startup time bits for AXP22X/AXP8XX PEKs

[Chen-Yu Tsai]

On Tue, Jul 18, 2017 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 18 Jul 2017, Chen-Yu Tsai wrote:
>
>> On Tue, Jul 18, 2017 at 3:25 PM, Quentin Schulz
>> <quentin.schulz@free-electrons.com> wrote:
>> > Hi Lee,
>> >
>> > On 18/07/2017 09:18, Lee Jones wrote:
>> >> On Mon, 17 Jul 2017, Quentin Schulz wrote:
>> >>
>> >>> Hi all,
>> >>>
>> >>> On 17/07/2017 11:53, Quentin Schulz wrote:
>> >>>> According to their datasheets, the AXP221, AXP223, AXP288, AXP803,
>> >>>> AXP809 and AXP813 PEK have different values for startup time bits from
>> >>>> the AXP20X PEK (which are currently used for all the aforementioned PMICs).
>> >>>>
>> >>>> This patch series adds support for platform_device_id to axp20x-pek driver
>> >>>> to support different startup time bits depending on PMIC's variant and set
>> >>>> the correct platform_device_id in the mfd.
>> >>>>
>> >>>> This has been tested on AXP813. To perform the test, modify the value of
>> >>>> startup and/or shutdown of the pek in sysfs. Press the button soldered to
>> >>>> PEK input of the AXP the amount of time you set in sysfs. The PMIC shuts
>> >>>> down the board without needing to unplug any power supply. Press PEK button
>> >>>> the amount of time you set in sysfs for startup and the PMIC will start the
>> >>>> board. Note that the time bits are obivoulsy hardware reset to their
>> >>>> default when you unplug all power supplies from the board.
>> >>>>
>> >>>
>> >>> I forgot to add that it depends on this patch series (particularly the
>> >>> third patch (3/6) "mfd: axp20x: Add support for AXP813 PMIC") from
>> >>> Chen-Yu:
>> >>> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1428284.html
>> >>
>> >> So you are going to re-send?
>> >>
>> >
>> > You put your acked-by just yesterday on this patch. Do I need to resend
>> > it anyway?
>> >
>> > The link I provided seems to be really slow/not working today, so I give
>> > you another one for reference: https://patchwork.kernel.org/patch/9803439/
>>
>> Or rather, would you like me to merge the AXP813 changes into my next
>> version, and you resend the rest for faster separate inclusion?
>
> What I'd really like is for one of you to collect up the patches which
> depend on each other (in the correct order of course), along with the
> Acks you've obtained and send as a single set.  That way, I am less
> likely to mess up the ordering when applying.

I'll include the mfd patch in my axp813 series.

Quentin, please send only the input driver for v2.

ChenYu

>
>> >>>> Quentin Schulz (2):
>> >>>>   Input: axp20x-pek: add support for AXP221 PEK
>> >>>>   mfd: axp20x: use correct platform device id for many PEK
>> >>>>
>> >>>>  drivers/input/misc/axp20x-pek.c | 62 ++++++++++++++++++++++++++++++++++-------
>> >>>>  drivers/mfd/axp20x.c            | 12 ++++----
>> >>>>  2 files changed, 58 insertions(+), 16 deletions(-)
>> >>>>
>> >>>
>> >>
>> >
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 0/2] use correct values for startup time bits for AXP22X/AXP8XX PEKs

[Lee Jones]

On Wed, 19 Jul 2017, Chen-Yu Tsai wrote:

> On Tue, Jul 18, 2017 at 5:53 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Tue, 18 Jul 2017, Chen-Yu Tsai wrote:
> >
> >> On Tue, Jul 18, 2017 at 3:25 PM, Quentin Schulz
> >> <quentin.schulz@free-electrons.com> wrote:
> >> > Hi Lee,
> >> >
> >> > On 18/07/2017 09:18, Lee Jones wrote:
> >> >> On Mon, 17 Jul 2017, Quentin Schulz wrote:
> >> >>
> >> >>> Hi all,
> >> >>>
> >> >>> On 17/07/2017 11:53, Quentin Schulz wrote:
> >> >>>> According to their datasheets, the AXP221, AXP223, AXP288, AXP803,
> >> >>>> AXP809 and AXP813 PEK have different values for startup time bits from
> >> >>>> the AXP20X PEK (which are currently used for all the aforementioned PMICs).
> >> >>>>
> >> >>>> This patch series adds support for platform_device_id to axp20x-pek driver
> >> >>>> to support different startup time bits depending on PMIC's variant and set
> >> >>>> the correct platform_device_id in the mfd.
> >> >>>>
> >> >>>> This has been tested on AXP813. To perform the test, modify the value of
> >> >>>> startup and/or shutdown of the pek in sysfs. Press the button soldered to
> >> >>>> PEK input of the AXP the amount of time you set in sysfs. The PMIC shuts
> >> >>>> down the board without needing to unplug any power supply. Press PEK button
> >> >>>> the amount of time you set in sysfs for startup and the PMIC will start the
> >> >>>> board. Note that the time bits are obivoulsy hardware reset to their
> >> >>>> default when you unplug all power supplies from the board.
> >> >>>>
> >> >>>
> >> >>> I forgot to add that it depends on this patch series (particularly the
> >> >>> third patch (3/6) "mfd: axp20x: Add support for AXP813 PMIC") from
> >> >>> Chen-Yu:
> >> >>> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1428284.html
> >> >>
> >> >> So you are going to re-send?
> >> >>
> >> >
> >> > You put your acked-by just yesterday on this patch. Do I need to resend
> >> > it anyway?
> >> >
> >> > The link I provided seems to be really slow/not working today, so I give
> >> > you another one for reference: https://patchwork.kernel.org/patch/9803439/
> >>
> >> Or rather, would you like me to merge the AXP813 changes into my next
> >> version, and you resend the rest for faster separate inclusion?
> >
> > What I'd really like is for one of you to collect up the patches which
> > depend on each other (in the correct order of course), along with the
> > Acks you've obtained and send as a single set.  That way, I am less
> > likely to mess up the ordering when applying.
> 
> I'll include the mfd patch in my axp813 series.

Wonderful, thank you!

> Quentin, please send only the input driver for v2.
> 
> ChenYu
> 
> >
> >> >>>> Quentin Schulz (2):
> >> >>>>   Input: axp20x-pek: add support for AXP221 PEK
> >> >>>>   mfd: axp20x: use correct platform device id for many PEK
> >> >>>>
> >> >>>>  drivers/input/misc/axp20x-pek.c | 62 ++++++++++++++++++++++++++++++++++-------
> >> >>>>  drivers/mfd/axp20x.c            | 12 ++++----
> >> >>>>  2 files changed, 58 insertions(+), 16 deletions(-)
> >> >>>>
> >> >>>
> >> >>
> >> >
> >

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog