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@xxxxxxxxxxxxxxxxxx>
> ---
> 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:
signature.asc
Description: PGP signature
