On Wed, Jul 19, 2017 at 09:43:35AM +0200, Quentin Schulz 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.
>
> Signed-off-by: Quentin Schulz <quentin.sch...@free-electrons.com>
> ---
> drivers/input/misc/axp20x-pek.c | 131
> +++++++++++++++++++++++++++-------------
> 1 file changed, 88 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/input/misc/axp20x-pek.c b/drivers/input/misc/axp20x-pek.c
> index 38c79ebff033..94c89fbeeeaa 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)
> + 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_show_ext_attr_startup(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
> +
> + return axp20x_show_ext_attr(dev, axp20x_pek->info->startup_time,
> + axp20x_pek->info->startup_mask, buf);
> +}
> +
> +static ssize_t axp20x_show_ext_attr_shutdown(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct axp20x_pek *axp20x_pek = dev_get_drvdata(dev);
> +
> + return axp20x_show_ext_attr(dev, axp20x_pek->info->shutdown_time,
> + axp20x_pek->info->shutdown_mask, buf);
> +}
> +
> static ssize_t axp20x_store_ext_attr(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf, size_t count)
> + 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,53 @@ 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_ext_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_ext_attr(dev, axp20x_pek->info->startup_time,
> + axp20x_pek->info->startup_mask, buf,
> + count);
> +}
> +
> +static ssize_t axp20x_store_ext_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_ext_attr(dev, axp20x_pek->info->shutdown_time,
> + axp20x_pek->info->shutdown_mask, buf,
> + count);
> +}
> +All these functions should be renamed, they're not extended attributes anymore. > +DEVICE_ATTR(startup, 0644, axp20x_show_ext_attr_startup, > axp20x_store_ext_attr_startup); > +DEVICE_ATTR(shutdown, 0644, axp20x_show_ext_attr_shutdown, > axp20x_store_ext_attr_shutdown); Both these lines generate a warning under checkpatch for a line too long. Please wrap them. Looks good otherwise, thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
signature.asc
Description: PGP signature
