[PATCH v3 0/2] add support for AXP22X/AXP288/AXP8XX PEK

[PATCH v3 0/2] add support for AXP22X/AXP288/AXP8XX 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 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.

v3:
  - name correctly the newly added functions (remove ext_ in their name),
  - since only two functions remained with an ext_ in their name (and
  since they are used by the newly added functions), rename it directly in
  the first patch (thus, remove the second patch of v2),
  - fix over-80-chars line checkpatch warning,

v2:
  - instead of duplicating all extended attributes, use the driver_data of
  platform_device_id to get startup and shutdown times and their respective
  masks,
  - remove extended attributes and use the structure storing startup and
  shutdown times and their respective masks,
  - separate in different patches,
  - removed mfd patch to correct mfd cell name since Chen-Yu will take care
  of it in another patch series,

Thanks,
Quentin

Quentin Schulz (2):
  Input: axp20x-pek: use driver_data of platform_device_id instead of extended attributes
  Input: axp20x-pek: add support for AXP221 PEK

 drivers/input/misc/axp20x-pek.c | 147 +++++++++++++++++++++++----------
 1 file changed, 104 insertions(+), 43 deletions(-)

base-commit: 22bbe310b60b37b029659460843265f190364e48
-- 
git-series 0.9.1

[PATCH v3 1/2] Input: axp20x-pek: use driver_data of platform_device_id instead of extended attributes

[Quentin Schulz]

To prepare an upcoming patch adding support for another PMIC that has
different startup and shutdown time, use driver_data of
platform_device_id instead of a fixed extended device attribute.

By doing so, we also remove a lot of nested structures that aren't
useful.

With this patch, a new PMIC can be easily supported by just filling
correctly its ax20x_info structure and adding a platform_device_id.

Moreover, since we get rid of extended attributes, rename
axp20x_store_ext_attr to axp20x_store_attr and axp20x_show_ext_attr to
axp20x_show_attr.

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

diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
index cfeb0e9..f616fe5 100644
--- a/drivers/input/misc/axp20x-pek.c
+++ b/drivers/input/misc/axp20x-pek.c
@@ -29,9 +29,17 @@
 #define AXP20X_PEK_STARTUP_MASK		(0xc0)
 #define AXP20X_PEK_SHUTDOWN_MASK	(0x03)
 
+struct axp20x_info {
+	const struct axp20x_time *startup_time;
+	unsigned int startup_mask;
+	const struct axp20x_time *shutdown_time;
+	unsigned int shutdown_mask;
+};
+
 struct axp20x_pek {
 	struct axp20x_dev *axp20x;
 	struct input_dev *input;
+	struct axp20x_info *info;
 	int irq_dbr;
 	int irq_dbf;
 };
@@ -55,31 +63,18 @@ static const struct axp20x_time shutdown_time[] = {
 	{ .time = 10000, .idx = 3 },
 };
 
-struct axp20x_pek_ext_attr {
-	const struct axp20x_time *p_time;
-	unsigned int mask;
-};
-
-static struct axp20x_pek_ext_attr axp20x_pek_startup_ext_attr = {
-	.p_time	= startup_time,
-	.mask	= AXP20X_PEK_STARTUP_MASK,
-};
-
-static struct axp20x_pek_ext_attr axp20x_pek_shutdown_ext_attr = {
-	.p_time	= shutdown_time,
-	.mask	= AXP20X_PEK_SHUTDOWN_MASK,
+static const struct axp20x_info axp20x_info = {
+	.startup_time = startup_time,
+	.startup_mask = AXP20X_PEK_STARTUP_MASK,
+	.shutdown_time = shutdown_time,
+	.shutdown_mask = AXP20X_PEK_SHUTDOWN_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;
-}
-
-static ssize_t axp20x_show_ext_attr(struct device *dev,
-				    struct device_attribute *attr, char *buf)
+static ssize_t axp20x_show_attr(struct device *dev,
+				const struct axp20x_time *time,
+				unsigned int mask, char *buf)
 {
 	struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
-	struct axp20x_pek_ext_attr *axp20x_ea = get_axp_ext_attr(attr);
 	unsigned int val;
 	int ret, i;
 
@@ -87,22 +82,42 @@ static ssize_t axp20x_show_ext_attr(struct device *dev,
 	if (ret != 0)
 		return ret;
 
-	val &= axp20x_ea->mask;
-	val >>= ffs(axp20x_ea->mask) - 1;
+	val &= mask;
+	val >>= ffs(mask) - 1;
 
 	for (i = 0; i < 4; i++)
-		if (val == axp20x_ea->p_time[i].idx)
-			val = axp20x_ea->p_time[i].time;
+		if (val == time[i].idx)
+			val = time[i].time;
 
 	return sprintf(buf, "%u\n", val);
 }
 
-static ssize_t axp20x_store_ext_attr(struct device *dev,
-				     struct device_attribute *attr,
-				     const char *buf, size_t count)
+static ssize_t axp20x_show_attr_startup(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
+
+	return axp20x_show_attr(dev, axp20x_pek->info->startup_time,
+				axp20x_pek->info->startup_mask, buf);
+}
+
+static ssize_t axp20x_show_attr_shutdown(struct device *dev,
+					 struct device_attribute *attr,
+					 char *buf)
+{
+	struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
+
+	return axp20x_show_attr(dev, axp20x_pek->info->shutdown_time,
+				axp20x_pek->info->shutdown_mask, buf);
+}
+
+static ssize_t axp20x_store_attr(struct device *dev,
+				 const struct axp20x_time *time,
+				 unsigned int mask, const char *buf,
+				 size_t count)
 {
 	struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
-	struct axp20x_pek_ext_attr *axp20x_ea = get_axp_ext_attr(attr);
 	char val_str[20];
 	size_t len;
 	int ret, i;
@@ -123,39 +138,52 @@ static ssize_t axp20x_store_ext_attr(struct device *dev,
 	for (i = 3; i >= 0; i--) {
 		unsigned int err;
 
-		err = abs(axp20x_ea->p_time[i].time - val);
+		err = abs(time[i].time - val);
 		if (err < best_err) {
 			best_err = err;
-			idx = axp20x_ea->p_time[i].idx;
+			idx = time[i].idx;
 		}
 
 		if (!err)
 			break;
 	}
 
-	idx <<= ffs(axp20x_ea->mask) - 1;
-	ret = regmap_update_bits(axp20x_pek->axp20x->regmap,
-				 AXP20X_PEK_KEY,
-				 axp20x_ea->mask, idx);
+	idx <<= ffs(mask) - 1;
+	ret = regmap_update_bits(axp20x_pek->axp20x->regmap, AXP20X_PEK_KEY,
+				 mask, idx);
 	if (ret != 0)
 		return -EINVAL;
 
 	return count;
 }
 
-static struct dev_ext_attribute axp20x_dev_attr_startup = {
-	.attr	= __ATTR(startup, 0644, axp20x_show_ext_attr, axp20x_store_ext_attr),
-	.var	= &axp20x_pek_startup_ext_attr,
-};
+static ssize_t axp20x_store_attr_startup(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf, size_t count)
+{
+	struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
 
-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,
-};
+	return axp20x_store_attr(dev, axp20x_pek->info->startup_time,
+				 axp20x_pek->info->startup_mask, buf, count);
+}
+
+static ssize_t axp20x_store_attr_shutdown(struct device *dev,
+					  struct device_attribute *attr,
+					  const char *buf, size_t count)
+{
+	struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
+
+	return axp20x_store_attr(dev, axp20x_pek->info->shutdown_time,
+				 axp20x_pek->info->shutdown_mask, buf, count);
+}
+
+DEVICE_ATTR(startup, 0644, axp20x_show_attr_startup, axp20x_store_attr_startup);
+DEVICE_ATTR(shutdown, 0644, axp20x_show_attr_shutdown,
+	    axp20x_store_attr_shutdown);
 
 static struct attribute *axp20x_attributes[] = {
-	&axp20x_dev_attr_startup.attr.attr,
-	&axp20x_dev_attr_shutdown.attr.attr,
+	&dev_attr_startup.attr,
+	&dev_attr_shutdown.attr,
 	NULL,
 };
 
@@ -291,8 +319,14 @@ static bool axp20x_pek_should_register_input(struct axp20x_pek *axp20x_pek,
 static int axp20x_pek_probe(struct platform_device *pdev)
 {
 	struct axp20x_pek *axp20x_pek;
+	const struct platform_device_id *match = platform_get_device_id(pdev);
 	int error;
 
+	if (!match) {
+		dev_err(&pdev->dev, "Failed to get platform_device_id\n");
+		return -EINVAL;
+	}
+
 	axp20x_pek = devm_kzalloc(&pdev->dev, sizeof(struct axp20x_pek),
 				  GFP_KERNEL);
 	if (!axp20x_pek)
@@ -306,6 +340,8 @@ static int axp20x_pek_probe(struct platform_device *pdev)
 			return error;
 	}
 
+	axp20x_pek->info = (struct axp20x_info *)match->driver_data;
+
 	error = devm_device_add_group(&pdev->dev, &axp20x_attribute_group);
 	if (error) {
 		dev_err(&pdev->dev, "Failed to create sysfs attributes: %d\n",
@@ -342,8 +378,16 @@ 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_info,
+	},
+};
+
 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,
-- 
git-series 0.9.1

Re: [PATCH v3 1/2] Input: axp20x-pek: use driver_data of platform_device_id instead of extended attributes

[Chen-Yu Tsai]

On Wed, Aug 9, 2017 at 5:45 PM, Quentin Schulz
<quentin.schulz@free-electrons.com> wrote:
> To prepare an upcoming patch adding support for another PMIC that has
> different startup and shutdown time, use driver_data of
> platform_device_id instead of a fixed extended device attribute.
>
> By doing so, we also remove a lot of nested structures that aren't
> useful.
>
> With this patch, a new PMIC can be easily supported by just filling
> correctly its ax20x_info structure and adding a platform_device_id.
>
> Moreover, since we get rid of extended attributes, rename
> axp20x_store_ext_attr to axp20x_store_attr and axp20x_show_ext_attr to
> axp20x_show_attr.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

[...]

> +
> +DEVICE_ATTR(startup, 0644, axp20x_show_attr_startup, axp20x_store_attr_startup);
> +DEVICE_ATTR(shutdown, 0644, axp20x_show_attr_shutdown,
> +           axp20x_store_attr_shutdown);
>

You could use DEVICE_ATTR_RW(...) if you renamed the function names to
fit the macro if you wanted.

Re: [PATCH v3 1/2] Input: axp20x-pek: use driver_data of platform_device_id instead of extended attributes

[Dmitry Torokhov]

On Mon, Aug 14, 2017 at 06:03:44PM +0800, Chen-Yu Tsai wrote:
> On Wed, Aug 9, 2017 at 5:45 PM, Quentin Schulz
> <quentin.schulz@free-electrons.com> wrote:
> > To prepare an upcoming patch adding support for another PMIC that has
> > different startup and shutdown time, use driver_data of
> > platform_device_id instead of a fixed extended device attribute.
> >
> > By doing so, we also remove a lot of nested structures that aren't
> > useful.
> >
> > With this patch, a new PMIC can be easily supported by just filling
> > correctly its ax20x_info structure and adding a platform_device_id.
> >
> > Moreover, since we get rid of extended attributes, rename
> > axp20x_store_ext_attr to axp20x_store_attr and axp20x_show_ext_attr to
> > axp20x_show_attr.
> >
> > Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> 
> Acked-by: Chen-Yu Tsai <wens@csie.org>

Applied, thank you.

> 
> [...]
> 
> > +
> > +DEVICE_ATTR(startup, 0644, axp20x_show_attr_startup, axp20x_store_attr_startup);
> > +DEVICE_ATTR(shutdown, 0644, axp20x_show_attr_shutdown,
> > +           axp20x_store_attr_shutdown);
> >
> 
> You could use DEVICE_ATTR_RW(...) if you renamed the function names to
> fit the macro if you wanted.

-- 
Dmitry

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

[Quentin Schulz]

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

Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/input/misc/axp20x-pek.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
index f616fe5..a542906 100644
--- a/drivers/input/misc/axp20x-pek.c
+++ b/drivers/input/misc/axp20x-pek.c
@@ -56,6 +56,13 @@ static const struct axp20x_time startup_time[] = {
 	{ .time = 2000, .idx = 3 },
 };
 
+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 shutdown_time[] = {
 	{ .time = 4000,  .idx = 0 },
 	{ .time = 6000,  .idx = 1 },
@@ -70,6 +77,13 @@ static const struct axp20x_info axp20x_info = {
 	.shutdown_mask = AXP20X_PEK_SHUTDOWN_MASK,
 };
 
+static const struct axp20x_info axp221_info = {
+	.startup_time = axp221_startup_time,
+	.startup_mask = AXP20X_PEK_STARTUP_MASK,
+	.shutdown_time = shutdown_time,
+	.shutdown_mask = AXP20X_PEK_SHUTDOWN_MASK,
+};
+
 static ssize_t axp20x_show_attr(struct device *dev,
 				const struct axp20x_time *time,
 				unsigned int mask, char *buf)
@@ -382,6 +396,9 @@ static const struct platform_device_id axp_pek_id_match[] = {
 	{
 		.name = "axp20x-pek",
 		.driver_data = (kernel_ulong_t)&axp20x_info,
+	}, {
+		.name = "axp221-pek",
+		.driver_data = (kernel_ulong_t)&axp221_info,
 	},
 };
 
-- 
git-series 0.9.1

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

[Dmitry Torokhov]

On Wed, Aug 09, 2017 at 11:45:55AM +0200, Quentin Schulz wrote:
> The AXP221 has different values for startup time bits from the AXP20X.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@free-electrons.com>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>

Applied, thank you.

> ---
>  drivers/input/misc/axp20x-pek.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
> index f616fe5..a542906 100644
> --- a/drivers/input/misc/axp20x-pek.c
> +++ b/drivers/input/misc/axp20x-pek.c
> @@ -56,6 +56,13 @@ static const struct axp20x_time startup_time[] = {
>  	{ .time = 2000, .idx = 3 },
>  };
>  
> +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 shutdown_time[] = {
>  	{ .time = 4000,  .idx = 0 },
>  	{ .time = 6000,  .idx = 1 },
> @@ -70,6 +77,13 @@ static const struct axp20x_info axp20x_info = {
>  	.shutdown_mask = AXP20X_PEK_SHUTDOWN_MASK,
>  };
>  
> +static const struct axp20x_info axp221_info = {
> +	.startup_time = axp221_startup_time,
> +	.startup_mask = AXP20X_PEK_STARTUP_MASK,
> +	.shutdown_time = shutdown_time,
> +	.shutdown_mask = AXP20X_PEK_SHUTDOWN_MASK,
> +};
> +
>  static ssize_t axp20x_show_attr(struct device *dev,
>  				const struct axp20x_time *time,
>  				unsigned int mask, char *buf)
> @@ -382,6 +396,9 @@ static const struct platform_device_id axp_pek_id_match[] = {
>  	{
>  		.name = "axp20x-pek",
>  		.driver_data = (kernel_ulong_t)&axp20x_info,
> +	}, {
> +		.name = "axp221-pek",
> +		.driver_data = (kernel_ulong_t)&axp221_info,
>  	},
>  };
>  
> -- 
> git-series 0.9.1

-- 
Dmitry