Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v3 3/5] devfreq: exynos-bus: convert to use dev_pm_opp_set_rate()
From: Kamil Konieczny @ 2019-07-25 15:15 UTC (permalink / raw)
  To: cwchoi00
  Cc: Chanwoo Choi, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
	Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
	MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
	Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
	Linux PM list, linux-samsung-soc
In-Reply-To: <CAGTfZH1OaYpTheQxWQs7Fs4qcJEGtXQHESLg0CJSsL=dmROpQw@mail.gmail.com>

Hi,

On 25.07.2019 16:53, Chanwoo Choi wrote:
> 2019년 7월 25일 (목) 오후 11:19, Kamil Konieczny
> <k.konieczny@partner.samsung.com>님이 작성:
>>
>> Hi Chanwoo,
>>
>> On 25.07.2019 12:17, Chanwoo Choi wrote:
>>> Hi Kamil,
>>>
>>> Looks good to me. But, I have some comment. Please check them.
>>
>> Thank you for review, please see answers below.
>>
>>> After this patch, exynos_bus_target is perfectly same with
>>> exynos_bus_passive_target. The exynos_bus_passive_target() could be removed.
>>
>> Ok, I will make it in separate patch to simplify review process.
> 
> I think you can just modify this patch without any separate patch.

Do you want me to send v5 with patch 5 squashed in patch 3 ?
 
>>> On 19. 7. 20. 오전 12:05, k.konieczny@partner.samsung.com wrote:
>>>> Reuse opp core code for setting bus clock and voltage. As a side
>>>> effect this allow useage of coupled regulators feature (required
>>>
>>> s/useage/usage ?
>>
>> Good catch, I will change it.
>>
>>>> for boards using Exynos5422/5800 SoCs) because dev_pm_opp_set_rate()
>>>> uses regulator_set_voltage_triplet() for setting regulator voltage
>>>> while the old code used regulator_set_voltage_tol() with fixed
>>>> tolerance. This patch also removes no longer needed parsing of DT
>>>> property "exynos,voltage-tolerance" (no Exynos devfreq DT node uses
>>>> it).
>>>>
>>>> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
>>>> ---
>>>>  drivers/devfreq/exynos-bus.c | 143 +++++++++--------------------------
>>>>  1 file changed, 37 insertions(+), 106 deletions(-)
>>>>
>>>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
>>>> index f391044aa39d..c2147b0912a0 100644
>>>> --- a/drivers/devfreq/exynos-bus.c
>>>> +++ b/drivers/devfreq/exynos-bus.c
>>>> @@ -25,7 +25,6 @@
>>>>  #include <linux/slab.h>
>>>>
>>>>  #define DEFAULT_SATURATION_RATIO    40
>>>> -#define DEFAULT_VOLTAGE_TOLERANCE   2
>>>>
>>>>  struct exynos_bus {
>>>>      struct device *dev;
>>>> @@ -37,9 +36,9 @@ struct exynos_bus {
>>>>
>>>>      unsigned long curr_freq;
>>>>
>>>> -    struct regulator *regulator;
>>>> +    struct opp_table *opp_table;
>>>> +
>>>>      struct clk *clk;
>>>> -    unsigned int voltage_tolerance;
>>>>      unsigned int ratio;
>>>>  };
>>>>
>>>> @@ -99,56 +98,23 @@ static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 flags)
>>>>  {
>>>>      struct exynos_bus *bus = dev_get_drvdata(dev);
>>>>      struct dev_pm_opp *new_opp;
>>>> -    unsigned long old_freq, new_freq, new_volt, tol;
>>>>      int ret = 0;
>>>>
>>>> -    /* Get new opp-bus instance according to new bus clock */
>>>> +    /* Get correct frequency for bus. */
>>>>      new_opp = devfreq_recommended_opp(dev, freq, flags);
>>>>      if (IS_ERR(new_opp)) {
>>>>              dev_err(dev, "failed to get recommended opp instance\n");
>>>>              return PTR_ERR(new_opp);
>>>>      }
>>>>
>>>> -    new_freq = dev_pm_opp_get_freq(new_opp);
>>>> -    new_volt = dev_pm_opp_get_voltage(new_opp);
>>>>      dev_pm_opp_put(new_opp);
>>>>
>>>> -    old_freq = bus->curr_freq;
>>>> -
>>>> -    if (old_freq == new_freq)
>>>> -            return 0;
>>>> -    tol = new_volt * bus->voltage_tolerance / 100;
>>>> -
>>>>      /* Change voltage and frequency according to new OPP level */
>>>>      mutex_lock(&bus->lock);
>>>> +    ret = dev_pm_opp_set_rate(dev, *freq);
>>>> +    if (!ret)
>>>> +            bus->curr_freq = *freq;
>>>>
>>>> -    if (old_freq < new_freq) {
>>>> -            ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
>>>> -            if (ret < 0) {
>>>> -                    dev_err(bus->dev, "failed to set voltage\n");
>>>> -                    goto out;
>>>> -            }
>>>> -    }
>>>> -
>>>> -    ret = clk_set_rate(bus->clk, new_freq);
>>>> -    if (ret < 0) {
>>>> -            dev_err(dev, "failed to change clock of bus\n");
>>>> -            clk_set_rate(bus->clk, old_freq);
>>>> -            goto out;
>>>> -    }
>>>> -
>>>> -    if (old_freq > new_freq) {
>>>> -            ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
>>>> -            if (ret < 0) {
>>>> -                    dev_err(bus->dev, "failed to set voltage\n");
>>>> -                    goto out;
>>>> -            }
>>>> -    }
>>>> -    bus->curr_freq = new_freq;
>>>> -
>>>> -    dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
>>>> -                    old_freq, new_freq, clk_get_rate(bus->clk));
>>>> -out:
>>>>      mutex_unlock(&bus->lock);
>>>>
>>>>      return ret;
>>>> @@ -195,8 +161,8 @@ static void exynos_bus_exit(struct device *dev)
>>>>              dev_warn(dev, "failed to disable the devfreq-event devices\n");
>>>>
>>>>      clk_disable_unprepare(bus->clk);
>>>> -    if (bus->regulator)
>>>> -            regulator_disable(bus->regulator);
>>>> +    if (bus->opp_table)
>>>> +            dev_pm_opp_put_regulators(bus->opp_table);
>>>>
>>>>      dev_pm_opp_of_remove_table(dev);
>>>>  }
>>>> @@ -209,39 +175,23 @@ static int exynos_bus_passive_target(struct device *dev, unsigned long *freq,
>>>>  {
>>>>      struct exynos_bus *bus = dev_get_drvdata(dev);
>>>>      struct dev_pm_opp *new_opp;
>>>> -    unsigned long old_freq, new_freq;
>>>> -    int ret = 0;
>>>> +    int ret;
>>>>
>>>> -    /* Get new opp-bus instance according to new bus clock */
>>>> +    /* Get correct frequency for bus. */
>>>>      new_opp = devfreq_recommended_opp(dev, freq, flags);
>>>>      if (IS_ERR(new_opp)) {
>>>>              dev_err(dev, "failed to get recommended opp instance\n");
>>>>              return PTR_ERR(new_opp);
>>>>      }
>>>>
>>>> -    new_freq = dev_pm_opp_get_freq(new_opp);
>>>>      dev_pm_opp_put(new_opp);
>>>>
>>>> -    old_freq = bus->curr_freq;
>>>> -
>>>> -    if (old_freq == new_freq)
>>>> -            return 0;
>>>> -
>>>>      /* Change the frequency according to new OPP level */
>>>>      mutex_lock(&bus->lock);
>>>> +    ret = dev_pm_opp_set_rate(dev, *freq);
>>>> +    if (!ret)
>>>> +            bus->curr_freq = *freq;
>>>>
>>>> -    ret = clk_set_rate(bus->clk, new_freq);
>>>> -    if (ret < 0) {
>>>> -            dev_err(dev, "failed to set the clock of bus\n");
>>>> -            goto out;
>>>> -    }
>>>> -
>>>> -    *freq = new_freq;
>>>> -    bus->curr_freq = new_freq;
>>>> -
>>>> -    dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
>>>> -                    old_freq, new_freq, clk_get_rate(bus->clk));
>>>> -out:
>>>>      mutex_unlock(&bus->lock);
>>>>
>>>>      return ret;
>>>> @@ -259,20 +209,9 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
>>>>                                      struct exynos_bus *bus)
>>>>  {
>>>>      struct device *dev = bus->dev;
>>>> -    int i, ret, count, size;
>>>> -
>>>> -    /* Get the regulator to provide each bus with the power */
>>>> -    bus->regulator = devm_regulator_get(dev, "vdd");
>>>> -    if (IS_ERR(bus->regulator)) {
>>>> -            dev_err(dev, "failed to get VDD regulator\n");
>>>> -            return PTR_ERR(bus->regulator);
>>>> -    }
>>>> -
>>>> -    ret = regulator_enable(bus->regulator);
>>>> -    if (ret < 0) {
>>>> -            dev_err(dev, "failed to enable VDD regulator\n");
>>>> -            return ret;
>>>> -    }
>>>> +    struct opp_table *opp_table;
>>>> +    const char *vdd = "vdd";
>>>> +    int i, count, size;
>>>>
>>>>      /*
>>>>       * Get the devfreq-event devices to get the current utilization of
>>>> @@ -281,26 +220,29 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
>>>>      count = devfreq_event_get_edev_count(dev);
>>>>      if (count < 0) {
>>>>              dev_err(dev, "failed to get the count of devfreq-event dev\n");
>>>> -            ret = count;
>>>> -            goto err_regulator;
>>>> +            return count;
>>>>      }
>>>> -    bus->edev_count = count;
>>>>
>>>> +    bus->edev_count = count;
>>>>      size = sizeof(*bus->edev) * count;
>>>>      bus->edev = devm_kzalloc(dev, size, GFP_KERNEL);
>>>> -    if (!bus->edev) {
>>>> -            ret = -ENOMEM;
>>>> -            goto err_regulator;
>>>> -    }
>>>> +    if (!bus->edev)
>>>> +            return -ENOMEM;
>>>>
>>>>      for (i = 0; i < count; i++) {
>>>>              bus->edev[i] = devfreq_event_get_edev_by_phandle(dev, i);
>>>> -            if (IS_ERR(bus->edev[i])) {
>>>> -                    ret = -EPROBE_DEFER;
>>>> -                    goto err_regulator;
>>>> -            }
>>>> +            if (IS_ERR(bus->edev[i]))
>>>> +                    return -EPROBE_DEFER;
>>>> +    }
>>>> +
>>>> +    opp_table = dev_pm_opp_set_regulators(dev, &vdd, 1);
>>>> +    if (IS_ERR(opp_table)) {
>>>> +            i = PTR_ERR(opp_table);
>>>> +            dev_err(dev, "failed to set regulators %d\n", i);
>>>> +            return i;
>>>
>>> Maybe, you just used the 'i' defined variable instead of adding
>>> new variable. But, I think that you better to add new variable
>>> like 'err' for the readability. Or, jut use the 'PTR_ERR(opp_table)'
>>> directly without any additional variable.
>>
>> I will remove not related changes, so here I will reuse 'ret' variable.
>>
>>>>      }
>>>>
>>>> +    bus->opp_table = opp_table;
>>>
>>> Add blank line.
>>
>> OK
>>
>>>>      /*
>>>>       * Optionally, Get the saturation ratio according to Exynos SoC
>>>>       * When measuring the utilization of each AXI bus with devfreq-event
>>>> @@ -314,16 +256,7 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
>>>>      if (of_property_read_u32(np, "exynos,saturation-ratio", &bus->ratio))
>>>>              bus->ratio = DEFAULT_SATURATION_RATIO;
>>>>
>>>> -    if (of_property_read_u32(np, "exynos,voltage-tolerance",
>>>> -                                    &bus->voltage_tolerance))
>>>> -            bus->voltage_tolerance = DEFAULT_VOLTAGE_TOLERANCE;
>>>> -
>>>>      return 0;
>>>> -
>>>> -err_regulator:
>>>> -    regulator_disable(bus->regulator);
>>>> -
>>>> -    return ret;
>>>>  }
>>>>
>>>>  static int exynos_bus_parse_of(struct exynos_bus *bus)
>>>> @@ -414,12 +347,8 @@ static int exynos_bus_probe(struct platform_device *pdev)
>>>>
>>>>      /* Parse the device-tree to get the resource information */
>>>>      ret = exynos_bus_parse_of(bus);
>>>> -    if (ret < 0) {
>>>> -            if (!passive)
>>>> -                    regulator_disable(bus->regulator);
>>>> -
>>>> -            return ret;
>>>> -    }
>>>> +    if (ret < 0)
>>>> +            goto err_reg;
>>>>
>>>>      if (passive)
>>>>              goto passive;
>>>> @@ -512,10 +441,12 @@ static int exynos_bus_probe(struct platform_device *pdev)
>>>>
>>>>  err:
>>>>      clk_disable_unprepare(bus->clk);
>>>> -    if (!passive)
>>>> -            regulator_disable(bus->regulator);
>>>> -
>>>>      dev_pm_opp_of_remove_table(dev);
>>>
>>> This function removes the 'opp_table'. But, the below code
>>> uses the 'opp_table' variable by dev_pm_opp_put_regulators().
>>>
>>> To disable the regulator, you have to call dev_pm_opp_of_remove_table(dev)
>>> after dev_pm_opp_put_regulators(bus->opp_table).
>>
>> Regulators should be set before dev_pm_opp_add_table(), so exit sequence
>> should be in reverse order,
>>
>> init order:
>>
>> exynos_bus_parent_parse_of()
>>         dev_pm_opp_set_regulators()
>> exynos_bus_parse_of()
>>         clk_prepare_enable()
>>         dev_pm_opp_of_add_table()
>>
>> exit or error order:
>>
>> dev_pm_opp_of_remove_table()
>> clk_disable_unprepare()
>> if (bus->opp_table)
>>         dev_pm_opp_put_regulators(bus->opp_table);
> 
> dev_pm_opp_of_remove_table() have to be called at the end of exit sequence
> after disabling clock and put regulators. Because dev_pm_opp_of_remove_table()
> frees the 'opp_table' pointer of device.
> 
> clk_disable_unprepare()
> if (bus->opp_table)
>           dev_pm_opp_put_regulators(bus->opp_table);
> dev_pm_opp_of_remove_table()

The table is reference counted so it will be freed after count go to zero.

>> I will send v4 soon.
>>
>>>> +err_reg:
>>>> +    if (bus->opp_table) {
>>>> +            dev_pm_opp_put_regulators(bus->opp_table);
>>>> +            bus->opp_table = NULL;
>>>> +    }
>>>>
>>>>      return ret;
>>>>  }

-- 
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland

^ permalink raw reply

* Re: [PATCH] arm64: dts: fast models: Increase clcd's max-memory-bandwidth
From: Robin Murphy @ 2019-07-25 15:15 UTC (permalink / raw)
  To: Kevin Brodsky, linux-arm-kernel
  Cc: devicetree, Pawel Moll, Ruben Ayrapetyan, Liviu Dudau, dri-devel,
	Sudeep Holla
In-Reply-To: <20190725145040.42182-1-kevin.brodsky@arm.com>

Hi Kevin,

On 25/07/2019 15:50, Kevin Brodsky wrote:
> It may be desirable on certain platforms, such as Android, to
> use 32bpp buffers. Since there is no clear bandwidth limit for the
> CLCD component on the fast model, let's increase
> max-memory-bandwidth to allow using 32bpp buffers.

Given that the property is optional anyway, would it hurt to just remove 
it? After trying to dig up any relevant internal email history, it's 
still far from clear how and why it got here in the first place.

Robin.

> Reported-by: Ruben Ayrapetyan <ruben.ayrapetyan@arm.com>
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---
>   arch/arm64/boot/dts/arm/fvp-base-revc.dts | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/arm/fvp-base-revc.dts b/arch/arm64/boot/dts/arm/fvp-base-revc.dts
> index 687707020ec1..3aee49ed6d88 100644
> --- a/arch/arm64/boot/dts/arm/fvp-base-revc.dts
> +++ b/arch/arm64/boot/dts/arm/fvp-base-revc.dts
> @@ -269,7 +269,7 @@
>   		motherboard {
>   			iofpga@3,00000000 {
>   				clcd@1f0000 {
> -					max-memory-bandwidth = <130000000>; /* 16bpp @ 63.5MHz */
> +					max-memory-bandwidth = <260000000>; /* 32bpp @ 63.5MHz */
>   				};
>   			};
>   		};
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH v4 2/5] devfreq: exynos-bus: convert to use dev_pm_opp_set_rate()
From: Chanwoo Choi @ 2019-07-25 15:08 UTC (permalink / raw)
  To: Kamil Konieczny
  Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
	Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
	MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
	Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
	Linux PM list, linux-samsung-soc
In-Reply-To: <20190725144300.25014-3-k.konieczny@partner.samsung.com>

Hi,

This patch has the same problem as I mentioned on v3.
We need to discuss it on v3[1]. Please check my reply.
[1] [PATCH v3 3/5] devfreq: exynos-bus: convert to use dev_pm_opp_set_rate()

2019년 7월 25일 (목) 오후 11:46, <k.konieczny@partner.samsung.com>님이 작성:
>
> Reuse opp core code for setting bus clock and voltage. As a side
> effect this allow usage of coupled regulators feature (required
> for boards using Exynos5422/5800 SoCs) because dev_pm_opp_set_rate()
> uses regulator_set_voltage_triplet() for setting regulator voltage
> while the old code used regulator_set_voltage_tol() with fixed
> tolerance. This patch also removes no longer needed parsing of DT
> property "exynos,voltage-tolerance" (no Exynos devfreq DT node uses
> it).
>
> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> ---
> Changes:
> v4:
> - remove unrelated changes, add newline before comment
>
> ---
>  drivers/devfreq/exynos-bus.c | 108 +++++++++--------------------------
>  1 file changed, 28 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index f34fa26f00d0..ebb8f46312b6 100644
> --- a/drivers/devfreq/exynos-bus.c
> +++ b/drivers/devfreq/exynos-bus.c
> @@ -25,7 +25,6 @@
>  #include <linux/slab.h>
>
>  #define DEFAULT_SATURATION_RATIO       40
> -#define DEFAULT_VOLTAGE_TOLERANCE      2
>
>  struct exynos_bus {
>         struct device *dev;
> @@ -37,9 +36,8 @@ struct exynos_bus {
>
>         unsigned long curr_freq;
>
> -       struct regulator *regulator;
> +       struct opp_table *opp_table;
>         struct clk *clk;
> -       unsigned int voltage_tolerance;
>         unsigned int ratio;
>  };
>
> @@ -99,56 +97,23 @@ static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 flags)
>  {
>         struct exynos_bus *bus = dev_get_drvdata(dev);
>         struct dev_pm_opp *new_opp;
> -       unsigned long old_freq, new_freq, new_volt, tol;
>         int ret = 0;
>
> -       /* Get new opp-bus instance according to new bus clock */
> +       /* Get correct frequency for bus. */
>         new_opp = devfreq_recommended_opp(dev, freq, flags);
>         if (IS_ERR(new_opp)) {
>                 dev_err(dev, "failed to get recommended opp instance\n");
>                 return PTR_ERR(new_opp);
>         }
>
> -       new_freq = dev_pm_opp_get_freq(new_opp);
> -       new_volt = dev_pm_opp_get_voltage(new_opp);
>         dev_pm_opp_put(new_opp);
>
> -       old_freq = bus->curr_freq;
> -
> -       if (old_freq == new_freq)
> -               return 0;
> -       tol = new_volt * bus->voltage_tolerance / 100;
> -
>         /* Change voltage and frequency according to new OPP level */
>         mutex_lock(&bus->lock);
> +       ret = dev_pm_opp_set_rate(dev, *freq);
> +       if (!ret)
> +               bus->curr_freq = *freq;
>
> -       if (old_freq < new_freq) {
> -               ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
> -               if (ret < 0) {
> -                       dev_err(bus->dev, "failed to set voltage\n");
> -                       goto out;
> -               }
> -       }
> -
> -       ret = clk_set_rate(bus->clk, new_freq);
> -       if (ret < 0) {
> -               dev_err(dev, "failed to change clock of bus\n");
> -               clk_set_rate(bus->clk, old_freq);
> -               goto out;
> -       }
> -
> -       if (old_freq > new_freq) {
> -               ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
> -               if (ret < 0) {
> -                       dev_err(bus->dev, "failed to set voltage\n");
> -                       goto out;
> -               }
> -       }
> -       bus->curr_freq = new_freq;
> -
> -       dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
> -                       old_freq, new_freq, clk_get_rate(bus->clk));
> -out:
>         mutex_unlock(&bus->lock);
>
>         return ret;
> @@ -196,8 +161,10 @@ static void exynos_bus_exit(struct device *dev)
>
>         dev_pm_opp_of_remove_table(dev);
>         clk_disable_unprepare(bus->clk);
> -       if (bus->regulator)
> -               regulator_disable(bus->regulator);
> +       if (bus->opp_table) {
> +               dev_pm_opp_put_regulators(bus->opp_table);
> +               bus->opp_table = NULL;
> +       }
>  }
>
>  /*
> @@ -208,39 +175,23 @@ static int exynos_bus_passive_target(struct device *dev, unsigned long *freq,
>  {
>         struct exynos_bus *bus = dev_get_drvdata(dev);
>         struct dev_pm_opp *new_opp;
> -       unsigned long old_freq, new_freq;
> -       int ret = 0;
> +       int ret;
>
> -       /* Get new opp-bus instance according to new bus clock */
> +       /* Get correct frequency for bus. */
>         new_opp = devfreq_recommended_opp(dev, freq, flags);
>         if (IS_ERR(new_opp)) {
>                 dev_err(dev, "failed to get recommended opp instance\n");
>                 return PTR_ERR(new_opp);
>         }
>
> -       new_freq = dev_pm_opp_get_freq(new_opp);
>         dev_pm_opp_put(new_opp);
>
> -       old_freq = bus->curr_freq;
> -
> -       if (old_freq == new_freq)
> -               return 0;
> -
>         /* Change the frequency according to new OPP level */
>         mutex_lock(&bus->lock);
> +       ret = dev_pm_opp_set_rate(dev, *freq);
> +       if (!ret)
> +               bus->curr_freq = *freq;
>
> -       ret = clk_set_rate(bus->clk, new_freq);
> -       if (ret < 0) {
> -               dev_err(dev, "failed to set the clock of bus\n");
> -               goto out;
> -       }
> -
> -       *freq = new_freq;
> -       bus->curr_freq = new_freq;
> -
> -       dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
> -                       old_freq, new_freq, clk_get_rate(bus->clk));
> -out:
>         mutex_unlock(&bus->lock);
>
>         return ret;
> @@ -258,21 +209,19 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
>                                         struct exynos_bus *bus)
>  {
>         struct device *dev = bus->dev;
> +       struct opp_table *opp_table;
> +       const char *vdd = "vdd";
>         int i, ret, count, size;
>
> -       /* Get the regulator to provide each bus with the power */
> -       bus->regulator = devm_regulator_get(dev, "vdd");
> -       if (IS_ERR(bus->regulator)) {
> -               dev_err(dev, "failed to get VDD regulator\n");
> -               return PTR_ERR(bus->regulator);
> -       }
> -
> -       ret = regulator_enable(bus->regulator);
> -       if (ret < 0) {
> -               dev_err(dev, "failed to enable VDD regulator\n");
> +       opp_table = dev_pm_opp_set_regulators(dev, &vdd, 1);
> +       if (IS_ERR(opp_table)) {
> +               ret = PTR_ERR(opp_table);
> +               dev_err(dev, "failed to set regulators %d\n", ret);
>                 return ret;
>         }
>
> +       bus->opp_table = opp_table;
> +
>         /*
>          * Get the devfreq-event devices to get the current utilization of
>          * buses. This raw data will be used in devfreq ondemand governor.
> @@ -313,14 +262,11 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
>         if (of_property_read_u32(np, "exynos,saturation-ratio", &bus->ratio))
>                 bus->ratio = DEFAULT_SATURATION_RATIO;
>
> -       if (of_property_read_u32(np, "exynos,voltage-tolerance",
> -                                       &bus->voltage_tolerance))
> -               bus->voltage_tolerance = DEFAULT_VOLTAGE_TOLERANCE;
> -
>         return 0;
>
>  err_regulator:
> -       regulator_disable(bus->regulator);
> +       dev_pm_opp_put_regulators(bus->opp_table);
> +       bus->opp_table = NULL;
>
>         return ret;
>  }
> @@ -511,8 +457,10 @@ static int exynos_bus_probe(struct platform_device *pdev)
>         dev_pm_opp_of_remove_table(dev);
>         clk_disable_unprepare(bus->clk);
>  err_reg:
> -       if (!passive)
> -               regulator_disable(bus->regulator);
> +       if (!passive) {
> +               dev_pm_opp_put_regulators(bus->opp_table);
> +               bus->opp_table = NULL;
> +       }
>
>         return ret;
>  }
> --
> 2.22.0
>


-- 
Best Regards,
Chanwoo Choi

^ permalink raw reply

* Re: [PATCH v4 1/5] devfreq: exynos-bus: correct clock enable sequence
From: Kamil Konieczny @ 2019-07-25 15:07 UTC (permalink / raw)
  To: cwchoi00
  Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
	Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
	MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
	Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
	Linux PM list, linux-samsung-soc
In-Reply-To: <CAGTfZH0=skWJ3Dny7voeRzDp5oRkbNO=Pf6j+PM03=epmX-86g@mail.gmail.com>

Hi,

On 25.07.2019 16:59, Chanwoo Choi wrote:
> Hi,
> 
> You are missing my Acked tag.

I changed code, so I can not add your Ack in v4.
Please send new Ack for this patch.

> 2019년 7월 25일 (목) 오후 11:44, <k.konieczny@partner.samsung.com>님이 작성:
>>
>> Regulators should be enabled before clocks to avoid h/w hang. This
>> require change in exynos_bus_probe() to move exynos_bus_parse_of()
>> after exynos_bus_parent_parse_of() and change in error handling.
>> Similar change is needed in exynos_bus_exit() where clock should be
>> disabled before regulators.
>>
>> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
>> ---
>> Changes:
>> v4:
>> - move regulator disable after clock disable
>> - remove unrelated changes
>> - add disabling regulators in error path in exynos_bus_probe()
>>
>> ---
>>  drivers/devfreq/exynos-bus.c | 31 +++++++++++++++++--------------
>>  1 file changed, 17 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
>> index 486cc5b422f1..f34fa26f00d0 100644
>> --- a/drivers/devfreq/exynos-bus.c
>> +++ b/drivers/devfreq/exynos-bus.c
>> @@ -194,11 +194,10 @@ static void exynos_bus_exit(struct device *dev)
>>         if (ret < 0)
>>                 dev_warn(dev, "failed to disable the devfreq-event devices\n");
>>
>> -       if (bus->regulator)
>> -               regulator_disable(bus->regulator);
>> -
>>         dev_pm_opp_of_remove_table(dev);
>>         clk_disable_unprepare(bus->clk);
>> +       if (bus->regulator)
>> +               regulator_disable(bus->regulator);
>>  }
>>
>>  /*
>> @@ -386,6 +385,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
>>         struct exynos_bus *bus;
>>         int ret, max_state;
>>         unsigned long min_freq, max_freq;
>> +       bool passive = false;
>>
>>         if (!np) {
>>                 dev_err(dev, "failed to find devicetree node\n");
>> @@ -399,27 +399,27 @@ static int exynos_bus_probe(struct platform_device *pdev)
>>         bus->dev = &pdev->dev;
>>         platform_set_drvdata(pdev, bus);
>>
>> -       /* Parse the device-tree to get the resource information */
>> -       ret = exynos_bus_parse_of(np, bus);
>> -       if (ret < 0)
>> -               return ret;
>> -
>>         profile = devm_kzalloc(dev, sizeof(*profile), GFP_KERNEL);
>> -       if (!profile) {
>> -               ret = -ENOMEM;
>> -               goto err;
>> -       }
>> +       if (!profile)
>> +               return -ENOMEM;
>>
>>         node = of_parse_phandle(dev->of_node, "devfreq", 0);
>>         if (node) {
>>                 of_node_put(node);
>> -               goto passive;
>> +               passive = true;
>>         } else {
>>                 ret = exynos_bus_parent_parse_of(np, bus);
>> +               if (ret < 0)
>> +                       return ret;
>>         }
>>
>> +       /* Parse the device-tree to get the resource information */
>> +       ret = exynos_bus_parse_of(np, bus);
>>         if (ret < 0)
>> -               goto err;
>> +               goto err_reg;
>> +
>> +       if (passive)
>> +               goto passive;
>>
>>         /* Initialize the struct profile and governor data for parent device */
>>         profile->polling_ms = 50;
>> @@ -510,6 +510,9 @@ static int exynos_bus_probe(struct platform_device *pdev)
>>  err:
>>         dev_pm_opp_of_remove_table(dev);
>>         clk_disable_unprepare(bus->clk);
>> +err_reg:
>> +       if (!passive)
>> +               regulator_disable(bus->regulator);
>>
>>         return ret;
>>  }
>> --
>> 2.22.0
>>
> 
> 

-- 
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland

^ permalink raw reply

* [PATCH V2 1/3] dt-bindings: net: dsa: ksz: document Microchip KSZ87xx family switches
From: Marek Vasut @ 2019-07-25 15:05 UTC (permalink / raw)
  To: netdev
  Cc: Marek Vasut, Andrew Lunn, David S . Miller, Florian Fainelli,
	Rob Herring, Tristram Ha, Vivien Didelot, Woojung Huh, devicetree
In-Reply-To: <20190725150552.6901-1-marex@denx.de>

Document Microchip KSZ87xx family switches. These include
KSZ8765 - 5 port switch
KSZ8794 - 4 port switch
KSZ8795 - 5 port switch

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: David S. Miller <davem@davemloft.net>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Tristram Ha <Tristram.Ha@microchip.com>
Cc: Vivien Didelot <vivien.didelot@gmail.com>
Cc: Woojung Huh <woojung.huh@microchip.com>
Cc: devicetree@vger.kernel.org
---
V2: No change
---
 Documentation/devicetree/bindings/net/dsa/ksz.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt b/Documentation/devicetree/bindings/net/dsa/ksz.txt
index 4ac21cef370e..5e8429b6f9ca 100644
--- a/Documentation/devicetree/bindings/net/dsa/ksz.txt
+++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt
@@ -5,6 +5,9 @@ Required properties:
 
 - compatible: For external switch chips, compatible string must be exactly one
   of the following:
+  - "microchip,ksz8765"
+  - "microchip,ksz8794"
+  - "microchip,ksz8795"
   - "microchip,ksz9477"
   - "microchip,ksz9897"
   - "microchip,ksz9896"
-- 
2.20.1

^ permalink raw reply related

* [v5 2/2] dt-bindings: mtd: Add Cadence NAND controller driver
From: Piotr Sroka @ 2019-07-25 14:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: David Woodhouse, BrianNorris, Boris Brezillon, Marek Vasut,
	Richard Weinberger, Rob Herring, Mark Rutland, linux-mtd,
	devicetree, Kazuhiro Kasai, Piotr Sroka
In-Reply-To: <20190725145804.8886-1-piotrs@cadence.com>

Document the bindings used by Cadence NAND controller driver

Signed-off-by: Piotr Sroka <piotrs@cadence.com>
---
Changes for v5:
- replace "_" by "-" in all properties
- change compatible name from cdns,hpnfc to cdns,hp-nfc
Changes for v4:
- add commit message
Changes for v3:
- add unit suffix for board_delay 
- move child description to proper place
- remove prefix cadence_ for reg and sdma fields
Changes for v2:
- remove chip dependends parameters from dts bindings
- add names for register ranges in dts bindings
- add generic bindings to describe NAND chip representation
---
 .../bindings/mtd/cadence-nand-controller.txt       | 50 ++++++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/cadence-nand-controller.txt

diff --git a/Documentation/devicetree/bindings/mtd/cadence-nand-controller.txt b/Documentation/devicetree/bindings/mtd/cadence-nand-controller.txt
new file mode 100644
index 000000000000..423547a3f993
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/cadence-nand-controller.txt
@@ -0,0 +1,50 @@
+* Cadence NAND controller
+
+Required properties:
+  - compatible : "cdns,hp-nfc"
+  - reg : Contains two entries, each of which is a tuple consisting of a
+	  physical address and length. The first entry is the address and
+	  length of the controller register set. The second entry is the
+	  address and length of the Slave DMA data port.
+  - reg-names: should contain "reg" and "sdma"
+  - interrupts : The interrupt number.
+  - clocks: phandle of the controller core clock (nf_clk).
+
+Optional properties:
+  - dmas: shall reference DMA channel associated to the NAND controller
+  - cdns,board-delay-ps : Estimated Board delay. The value includes the total
+    round trip delay for the signals and is used for deciding on values
+    associated with data read capture. The example formula for SDR mode is
+    the following:
+    board delay = RE#PAD delay + PCB trace to device + PCB trace from device
+    + DQ PAD delay
+
+Child nodes represent the available NAND chips.
+
+Required properties of NAND chips:
+  - reg: shall contain the native Chip Select ids from 0 to max supported by
+    the cadence nand flash controller
+
+
+See Documentation/devicetree/bindings/mtd/nand.txt for more details on
+generic bindings.
+
+Example:
+
+nand_controller: nand-controller @60000000 {
+	  compatible = "cdns,hp-nfc";
+	  reg = <0x60000000 0x10000>, <0x80000000 0x10000>;
+	  reg-names = "reg", "sdma";
+	  clocks = <&nf_clk>;
+	  cdns,board-delay-ps = <4830>;
+	  interrupts = <2 0>;
+	  nand@0 {
+	      reg = <0>;
+	      label = "nand-1";
+	  };
+	  nand@1 {
+	      reg = <1>;
+	      label = "nand-2";
+	  };
+
+};
-- 
2.15.0

^ permalink raw reply related

* Re: [PATCH v4 1/5] devfreq: exynos-bus: correct clock enable sequence
From: Chanwoo Choi @ 2019-07-25 14:59 UTC (permalink / raw)
  To: Kamil Konieczny
  Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
	Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
	MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
	Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
	Linux PM list, linux-samsung-soc
In-Reply-To: <20190725144300.25014-2-k.konieczny@partner.samsung.com>

Hi,

You are missing my Acked tag.

2019년 7월 25일 (목) 오후 11:44, <k.konieczny@partner.samsung.com>님이 작성:
>
> Regulators should be enabled before clocks to avoid h/w hang. This
> require change in exynos_bus_probe() to move exynos_bus_parse_of()
> after exynos_bus_parent_parse_of() and change in error handling.
> Similar change is needed in exynos_bus_exit() where clock should be
> disabled before regulators.
>
> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> ---
> Changes:
> v4:
> - move regulator disable after clock disable
> - remove unrelated changes
> - add disabling regulators in error path in exynos_bus_probe()
>
> ---
>  drivers/devfreq/exynos-bus.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index 486cc5b422f1..f34fa26f00d0 100644
> --- a/drivers/devfreq/exynos-bus.c
> +++ b/drivers/devfreq/exynos-bus.c
> @@ -194,11 +194,10 @@ static void exynos_bus_exit(struct device *dev)
>         if (ret < 0)
>                 dev_warn(dev, "failed to disable the devfreq-event devices\n");
>
> -       if (bus->regulator)
> -               regulator_disable(bus->regulator);
> -
>         dev_pm_opp_of_remove_table(dev);
>         clk_disable_unprepare(bus->clk);
> +       if (bus->regulator)
> +               regulator_disable(bus->regulator);
>  }
>
>  /*
> @@ -386,6 +385,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
>         struct exynos_bus *bus;
>         int ret, max_state;
>         unsigned long min_freq, max_freq;
> +       bool passive = false;
>
>         if (!np) {
>                 dev_err(dev, "failed to find devicetree node\n");
> @@ -399,27 +399,27 @@ static int exynos_bus_probe(struct platform_device *pdev)
>         bus->dev = &pdev->dev;
>         platform_set_drvdata(pdev, bus);
>
> -       /* Parse the device-tree to get the resource information */
> -       ret = exynos_bus_parse_of(np, bus);
> -       if (ret < 0)
> -               return ret;
> -
>         profile = devm_kzalloc(dev, sizeof(*profile), GFP_KERNEL);
> -       if (!profile) {
> -               ret = -ENOMEM;
> -               goto err;
> -       }
> +       if (!profile)
> +               return -ENOMEM;
>
>         node = of_parse_phandle(dev->of_node, "devfreq", 0);
>         if (node) {
>                 of_node_put(node);
> -               goto passive;
> +               passive = true;
>         } else {
>                 ret = exynos_bus_parent_parse_of(np, bus);
> +               if (ret < 0)
> +                       return ret;
>         }
>
> +       /* Parse the device-tree to get the resource information */
> +       ret = exynos_bus_parse_of(np, bus);
>         if (ret < 0)
> -               goto err;
> +               goto err_reg;
> +
> +       if (passive)
> +               goto passive;
>
>         /* Initialize the struct profile and governor data for parent device */
>         profile->polling_ms = 50;
> @@ -510,6 +510,9 @@ static int exynos_bus_probe(struct platform_device *pdev)
>  err:
>         dev_pm_opp_of_remove_table(dev);
>         clk_disable_unprepare(bus->clk);
> +err_reg:
> +       if (!passive)
> +               regulator_disable(bus->regulator);
>
>         return ret;
>  }
> --
> 2.22.0
>


-- 
Best Regards,
Chanwoo Choi

^ permalink raw reply

* Re: [PATCH v3 3/5] devfreq: exynos-bus: convert to use dev_pm_opp_set_rate()
From: Chanwoo Choi @ 2019-07-25 14:53 UTC (permalink / raw)
  To: Kamil Konieczny
  Cc: Chanwoo Choi, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
	Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
	MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
	Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
	Linux PM list, linux-samsung-soc
In-Reply-To: <ed80b6e3-5b40-18ce-ca1e-65520edf516e@partner.samsung.com>

2019년 7월 25일 (목) 오후 11:19, Kamil Konieczny
<k.konieczny@partner.samsung.com>님이 작성:
>
> Hi Chanwoo,
>
> On 25.07.2019 12:17, Chanwoo Choi wrote:
> > Hi Kamil,
> >
> > Looks good to me. But, I have some comment. Please check them.
>
> Thank you for review, please see answers below.
>
> > After this patch, exynos_bus_target is perfectly same with
> > exynos_bus_passive_target. The exynos_bus_passive_target() could be removed.
>
> Ok, I will make it in separate patch to simplify review process.

I think you can just modify this patch without any separate patch.

> > On 19. 7. 20. 오전 12:05, k.konieczny@partner.samsung.com wrote:
> >> Reuse opp core code for setting bus clock and voltage. As a side
> >> effect this allow useage of coupled regulators feature (required
> >
> > s/useage/usage ?
>
> Good catch, I will change it.
>
> >> for boards using Exynos5422/5800 SoCs) because dev_pm_opp_set_rate()
> >> uses regulator_set_voltage_triplet() for setting regulator voltage
> >> while the old code used regulator_set_voltage_tol() with fixed
> >> tolerance. This patch also removes no longer needed parsing of DT
> >> property "exynos,voltage-tolerance" (no Exynos devfreq DT node uses
> >> it).
> >>
> >> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> >> ---
> >>  drivers/devfreq/exynos-bus.c | 143 +++++++++--------------------------
> >>  1 file changed, 37 insertions(+), 106 deletions(-)
> >>
> >> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> >> index f391044aa39d..c2147b0912a0 100644
> >> --- a/drivers/devfreq/exynos-bus.c
> >> +++ b/drivers/devfreq/exynos-bus.c
> >> @@ -25,7 +25,6 @@
> >>  #include <linux/slab.h>
> >>
> >>  #define DEFAULT_SATURATION_RATIO    40
> >> -#define DEFAULT_VOLTAGE_TOLERANCE   2
> >>
> >>  struct exynos_bus {
> >>      struct device *dev;
> >> @@ -37,9 +36,9 @@ struct exynos_bus {
> >>
> >>      unsigned long curr_freq;
> >>
> >> -    struct regulator *regulator;
> >> +    struct opp_table *opp_table;
> >> +
> >>      struct clk *clk;
> >> -    unsigned int voltage_tolerance;
> >>      unsigned int ratio;
> >>  };
> >>
> >> @@ -99,56 +98,23 @@ static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 flags)
> >>  {
> >>      struct exynos_bus *bus = dev_get_drvdata(dev);
> >>      struct dev_pm_opp *new_opp;
> >> -    unsigned long old_freq, new_freq, new_volt, tol;
> >>      int ret = 0;
> >>
> >> -    /* Get new opp-bus instance according to new bus clock */
> >> +    /* Get correct frequency for bus. */
> >>      new_opp = devfreq_recommended_opp(dev, freq, flags);
> >>      if (IS_ERR(new_opp)) {
> >>              dev_err(dev, "failed to get recommended opp instance\n");
> >>              return PTR_ERR(new_opp);
> >>      }
> >>
> >> -    new_freq = dev_pm_opp_get_freq(new_opp);
> >> -    new_volt = dev_pm_opp_get_voltage(new_opp);
> >>      dev_pm_opp_put(new_opp);
> >>
> >> -    old_freq = bus->curr_freq;
> >> -
> >> -    if (old_freq == new_freq)
> >> -            return 0;
> >> -    tol = new_volt * bus->voltage_tolerance / 100;
> >> -
> >>      /* Change voltage and frequency according to new OPP level */
> >>      mutex_lock(&bus->lock);
> >> +    ret = dev_pm_opp_set_rate(dev, *freq);
> >> +    if (!ret)
> >> +            bus->curr_freq = *freq;
> >>
> >> -    if (old_freq < new_freq) {
> >> -            ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
> >> -            if (ret < 0) {
> >> -                    dev_err(bus->dev, "failed to set voltage\n");
> >> -                    goto out;
> >> -            }
> >> -    }
> >> -
> >> -    ret = clk_set_rate(bus->clk, new_freq);
> >> -    if (ret < 0) {
> >> -            dev_err(dev, "failed to change clock of bus\n");
> >> -            clk_set_rate(bus->clk, old_freq);
> >> -            goto out;
> >> -    }
> >> -
> >> -    if (old_freq > new_freq) {
> >> -            ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
> >> -            if (ret < 0) {
> >> -                    dev_err(bus->dev, "failed to set voltage\n");
> >> -                    goto out;
> >> -            }
> >> -    }
> >> -    bus->curr_freq = new_freq;
> >> -
> >> -    dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
> >> -                    old_freq, new_freq, clk_get_rate(bus->clk));
> >> -out:
> >>      mutex_unlock(&bus->lock);
> >>
> >>      return ret;
> >> @@ -195,8 +161,8 @@ static void exynos_bus_exit(struct device *dev)
> >>              dev_warn(dev, "failed to disable the devfreq-event devices\n");
> >>
> >>      clk_disable_unprepare(bus->clk);
> >> -    if (bus->regulator)
> >> -            regulator_disable(bus->regulator);
> >> +    if (bus->opp_table)
> >> +            dev_pm_opp_put_regulators(bus->opp_table);
> >>
> >>      dev_pm_opp_of_remove_table(dev);
> >>  }
> >> @@ -209,39 +175,23 @@ static int exynos_bus_passive_target(struct device *dev, unsigned long *freq,
> >>  {
> >>      struct exynos_bus *bus = dev_get_drvdata(dev);
> >>      struct dev_pm_opp *new_opp;
> >> -    unsigned long old_freq, new_freq;
> >> -    int ret = 0;
> >> +    int ret;
> >>
> >> -    /* Get new opp-bus instance according to new bus clock */
> >> +    /* Get correct frequency for bus. */
> >>      new_opp = devfreq_recommended_opp(dev, freq, flags);
> >>      if (IS_ERR(new_opp)) {
> >>              dev_err(dev, "failed to get recommended opp instance\n");
> >>              return PTR_ERR(new_opp);
> >>      }
> >>
> >> -    new_freq = dev_pm_opp_get_freq(new_opp);
> >>      dev_pm_opp_put(new_opp);
> >>
> >> -    old_freq = bus->curr_freq;
> >> -
> >> -    if (old_freq == new_freq)
> >> -            return 0;
> >> -
> >>      /* Change the frequency according to new OPP level */
> >>      mutex_lock(&bus->lock);
> >> +    ret = dev_pm_opp_set_rate(dev, *freq);
> >> +    if (!ret)
> >> +            bus->curr_freq = *freq;
> >>
> >> -    ret = clk_set_rate(bus->clk, new_freq);
> >> -    if (ret < 0) {
> >> -            dev_err(dev, "failed to set the clock of bus\n");
> >> -            goto out;
> >> -    }
> >> -
> >> -    *freq = new_freq;
> >> -    bus->curr_freq = new_freq;
> >> -
> >> -    dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
> >> -                    old_freq, new_freq, clk_get_rate(bus->clk));
> >> -out:
> >>      mutex_unlock(&bus->lock);
> >>
> >>      return ret;
> >> @@ -259,20 +209,9 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
> >>                                      struct exynos_bus *bus)
> >>  {
> >>      struct device *dev = bus->dev;
> >> -    int i, ret, count, size;
> >> -
> >> -    /* Get the regulator to provide each bus with the power */
> >> -    bus->regulator = devm_regulator_get(dev, "vdd");
> >> -    if (IS_ERR(bus->regulator)) {
> >> -            dev_err(dev, "failed to get VDD regulator\n");
> >> -            return PTR_ERR(bus->regulator);
> >> -    }
> >> -
> >> -    ret = regulator_enable(bus->regulator);
> >> -    if (ret < 0) {
> >> -            dev_err(dev, "failed to enable VDD regulator\n");
> >> -            return ret;
> >> -    }
> >> +    struct opp_table *opp_table;
> >> +    const char *vdd = "vdd";
> >> +    int i, count, size;
> >>
> >>      /*
> >>       * Get the devfreq-event devices to get the current utilization of
> >> @@ -281,26 +220,29 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
> >>      count = devfreq_event_get_edev_count(dev);
> >>      if (count < 0) {
> >>              dev_err(dev, "failed to get the count of devfreq-event dev\n");
> >> -            ret = count;
> >> -            goto err_regulator;
> >> +            return count;
> >>      }
> >> -    bus->edev_count = count;
> >>
> >> +    bus->edev_count = count;
> >>      size = sizeof(*bus->edev) * count;
> >>      bus->edev = devm_kzalloc(dev, size, GFP_KERNEL);
> >> -    if (!bus->edev) {
> >> -            ret = -ENOMEM;
> >> -            goto err_regulator;
> >> -    }
> >> +    if (!bus->edev)
> >> +            return -ENOMEM;
> >>
> >>      for (i = 0; i < count; i++) {
> >>              bus->edev[i] = devfreq_event_get_edev_by_phandle(dev, i);
> >> -            if (IS_ERR(bus->edev[i])) {
> >> -                    ret = -EPROBE_DEFER;
> >> -                    goto err_regulator;
> >> -            }
> >> +            if (IS_ERR(bus->edev[i]))
> >> +                    return -EPROBE_DEFER;
> >> +    }
> >> +
> >> +    opp_table = dev_pm_opp_set_regulators(dev, &vdd, 1);
> >> +    if (IS_ERR(opp_table)) {
> >> +            i = PTR_ERR(opp_table);
> >> +            dev_err(dev, "failed to set regulators %d\n", i);
> >> +            return i;
> >
> > Maybe, you just used the 'i' defined variable instead of adding
> > new variable. But, I think that you better to add new variable
> > like 'err' for the readability. Or, jut use the 'PTR_ERR(opp_table)'
> > directly without any additional variable.
>
> I will remove not related changes, so here I will reuse 'ret' variable.
>
> >>      }
> >>
> >> +    bus->opp_table = opp_table;
> >
> > Add blank line.
>
> OK
>
> >>      /*
> >>       * Optionally, Get the saturation ratio according to Exynos SoC
> >>       * When measuring the utilization of each AXI bus with devfreq-event
> >> @@ -314,16 +256,7 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
> >>      if (of_property_read_u32(np, "exynos,saturation-ratio", &bus->ratio))
> >>              bus->ratio = DEFAULT_SATURATION_RATIO;
> >>
> >> -    if (of_property_read_u32(np, "exynos,voltage-tolerance",
> >> -                                    &bus->voltage_tolerance))
> >> -            bus->voltage_tolerance = DEFAULT_VOLTAGE_TOLERANCE;
> >> -
> >>      return 0;
> >> -
> >> -err_regulator:
> >> -    regulator_disable(bus->regulator);
> >> -
> >> -    return ret;
> >>  }
> >>
> >>  static int exynos_bus_parse_of(struct exynos_bus *bus)
> >> @@ -414,12 +347,8 @@ static int exynos_bus_probe(struct platform_device *pdev)
> >>
> >>      /* Parse the device-tree to get the resource information */
> >>      ret = exynos_bus_parse_of(bus);
> >> -    if (ret < 0) {
> >> -            if (!passive)
> >> -                    regulator_disable(bus->regulator);
> >> -
> >> -            return ret;
> >> -    }
> >> +    if (ret < 0)
> >> +            goto err_reg;
> >>
> >>      if (passive)
> >>              goto passive;
> >> @@ -512,10 +441,12 @@ static int exynos_bus_probe(struct platform_device *pdev)
> >>
> >>  err:
> >>      clk_disable_unprepare(bus->clk);
> >> -    if (!passive)
> >> -            regulator_disable(bus->regulator);
> >> -
> >>      dev_pm_opp_of_remove_table(dev);
> >
> > This function removes the 'opp_table'. But, the below code
> > uses the 'opp_table' variable by dev_pm_opp_put_regulators().
> >
> > To disable the regulator, you have to call dev_pm_opp_of_remove_table(dev)
> > after dev_pm_opp_put_regulators(bus->opp_table).
>
> Regulators should be set before dev_pm_opp_add_table(), so exit sequence
> should be in reverse order,
>
> init order:
>
> exynos_bus_parent_parse_of()
>         dev_pm_opp_set_regulators()
> exynos_bus_parse_of()
>         clk_prepare_enable()
>         dev_pm_opp_of_add_table()
>
> exit or error order:
>
> dev_pm_opp_of_remove_table()
> clk_disable_unprepare()
> if (bus->opp_table)
>         dev_pm_opp_put_regulators(bus->opp_table);

dev_pm_opp_of_remove_table() have to be called at the end of exit sequence
after disabling clock and put regulators. Because dev_pm_opp_of_remove_table()
frees the 'opp_table' pointer of device.

clk_disable_unprepare()
if (bus->opp_table)
          dev_pm_opp_put_regulators(bus->opp_table);
dev_pm_opp_of_remove_table()

>
> I will send v4 soon.
>
> >> +err_reg:
> >> +    if (bus->opp_table) {
> >> +            dev_pm_opp_put_regulators(bus->opp_table);
> >> +            bus->opp_table = NULL;
> >> +    }
> >>
> >>      return ret;
> >>  }
>
> --
> Best regards,
> Kamil Konieczny
> Samsung R&D Institute Poland
>


-- 
Best Regards,
Chanwoo Choi

^ permalink raw reply

* [PATCH] arm64: dts: fast models: Increase clcd's max-memory-bandwidth
From: Kevin Brodsky @ 2019-07-25 14:50 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: devicetree, Pawel Moll, Ruben Ayrapetyan, Linus Walleij,
	Kevin Brodsky, Liviu Dudau, dri-devel, Sudeep Holla,
	Brian Starkey

It may be desirable on certain platforms, such as Android, to
use 32bpp buffers. Since there is no clear bandwidth limit for the
CLCD component on the fast model, let's increase
max-memory-bandwidth to allow using 32bpp buffers.

Reported-by: Ruben Ayrapetyan <ruben.ayrapetyan@arm.com>
Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
 arch/arm64/boot/dts/arm/fvp-base-revc.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/arm/fvp-base-revc.dts b/arch/arm64/boot/dts/arm/fvp-base-revc.dts
index 687707020ec1..3aee49ed6d88 100644
--- a/arch/arm64/boot/dts/arm/fvp-base-revc.dts
+++ b/arch/arm64/boot/dts/arm/fvp-base-revc.dts
@@ -269,7 +269,7 @@
 		motherboard {
 			iofpga@3,00000000 {
 				clcd@1f0000 {
-					max-memory-bandwidth = <130000000>; /* 16bpp @ 63.5MHz */
+					max-memory-bandwidth = <260000000>; /* 32bpp @ 63.5MHz */
 				};
 			};
 		};
-- 
2.22.0

^ permalink raw reply related

* Re: [PATCH net-next v2 3/4] dt-bindings: net: fsl: enetc: Add bindings for the central MDIO PCIe endpoint
From: Sergei Shtylyov @ 2019-07-25 14:49 UTC (permalink / raw)
  To: Claudiu Manoil, David S . Miller
  Cc: andrew, Rob Herring, Li Yang, alexandru.marginean, netdev,
	devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1564053568-20522-4-git-send-email-claudiu.manoil@nxp.com>

Hello!

On 07/25/2019 02:19 PM, Claudiu Manoil wrote:

> The on-chip PCIe root complex that integrates the ENETC ethernet
> controllers also integrates a PCIe enpoint for the MDIO controller
> provinding for cetralized control of the ENETC mdio bus.

   Providing, centralized.

> Add bindings for this "central" MDIO Integrated PCIe Endpoit.
> 
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
> ---
> v1 - none
> v2 - none
> 
>  .../devicetree/bindings/net/fsl-enetc.txt     | 42 +++++++++++++++++--
>  1 file changed, 39 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/fsl-enetc.txt b/Documentation/devicetree/bindings/net/fsl-enetc.txt
> index 25fc687419db..c090f6df7a39 100644
> --- a/Documentation/devicetree/bindings/net/fsl-enetc.txt
> +++ b/Documentation/devicetree/bindings/net/fsl-enetc.txt
[...]
> @@ -47,8 +49,42 @@ Example:
>  		};
>  	};
>  
> -2) The ENETC port is an internal port or has a fixed-link external
> -connection:
> +1.2. Using the central MDIO PCIe enpoint device

   Endpoint. -ETOOMANYTYPOS. :-)

[...]

MBR, Sergei

^ permalink raw reply

* [PATCH v4 5/5] devfreq: exynos-bus: remove exynos_bus_passive_target()
From: k.konieczny @ 2019-07-25 14:43 UTC (permalink / raw)
  To: k.konieczny
  Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
	Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
	MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
	Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
	linux-pm, linux-samsung-soc
In-Reply-To: <20190725144300.25014-1-k.konieczny@partner.samsung.com>

Both functions exynos_bus_passive_target() and exynos_bus_target()
have the same code, so remove exynos_bus_passive_target(). In
exynos_bus_probe() replace it with exynos_bus_target.

Suggested-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
---
This patch is new to this series.

---
 drivers/devfreq/exynos-bus.c | 34 ++--------------------------------
 1 file changed, 2 insertions(+), 32 deletions(-)

diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index ebb8f46312b6..2aeb6cc07960 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -91,7 +91,7 @@ static int exynos_bus_get_event(struct exynos_bus *bus,
 }
 
 /*
- * Must necessary function for devfreq simple-ondemand governor
+ * devfreq function for both simple-ondemand and passive governor
  */
 static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 flags)
 {
@@ -167,36 +167,6 @@ static void exynos_bus_exit(struct device *dev)
 	}
 }
 
-/*
- * Must necessary function for devfreq passive governor
- */
-static int exynos_bus_passive_target(struct device *dev, unsigned long *freq,
-					u32 flags)
-{
-	struct exynos_bus *bus = dev_get_drvdata(dev);
-	struct dev_pm_opp *new_opp;
-	int ret;
-
-	/* Get correct frequency for bus. */
-	new_opp = devfreq_recommended_opp(dev, freq, flags);
-	if (IS_ERR(new_opp)) {
-		dev_err(dev, "failed to get recommended opp instance\n");
-		return PTR_ERR(new_opp);
-	}
-
-	dev_pm_opp_put(new_opp);
-
-	/* Change the frequency according to new OPP level */
-	mutex_lock(&bus->lock);
-	ret = dev_pm_opp_set_rate(dev, *freq);
-	if (!ret)
-		bus->curr_freq = *freq;
-
-	mutex_unlock(&bus->lock);
-
-	return ret;
-}
-
 static void exynos_bus_passive_exit(struct device *dev)
 {
 	struct exynos_bus *bus = dev_get_drvdata(dev);
@@ -417,7 +387,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
 	goto out;
 passive:
 	/* Initialize the struct profile and governor data for passive device */
-	profile->target = exynos_bus_passive_target;
+	profile->target = exynos_bus_target;
 	profile->exit = exynos_bus_passive_exit;
 
 	/* Get the instance of parent devfreq device */
-- 
2.22.0

^ permalink raw reply related

* [PATCH v4 4/5] dt-bindings: devfreq: exynos-bus: remove unused property
From: k.konieczny @ 2019-07-25 14:42 UTC (permalink / raw)
  To: k.konieczny
  Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
	Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
	MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
	Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
	linux-pm, linux-samsung-soc
In-Reply-To: <20190725144300.25014-1-k.konieczny@partner.samsung.com>

Remove unused DT property "exynos,voltage-tolerance".

Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 Documentation/devicetree/bindings/devfreq/exynos-bus.txt | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
index f8e946471a58..e71f752cc18f 100644
--- a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
+++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
@@ -50,8 +50,6 @@ Required properties only for passive bus device:
 Optional properties only for parent bus device:
 - exynos,saturation-ratio: the percentage value which is used to calibrate
 			the performance count against total cycle count.
-- exynos,voltage-tolerance: the percentage value for bus voltage tolerance
-			which is used to calculate the max voltage.
 
 Detailed correlation between sub-blocks and power line according to Exynos SoC:
 - In case of Exynos3250, there are two power line as following:
-- 
2.22.0

^ permalink raw reply related

* [PATCH v4 3/5] ARM: dts: exynos: add initial data for coupled regulators for Exynos5422/5800
From: k.konieczny @ 2019-07-25 14:42 UTC (permalink / raw)
  To: k.konieczny
  Cc: Marek Szyprowski, Bartlomiej Zolnierkiewicz, Chanwoo Choi,
	Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
	MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
	Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
	linux-pm, linux-samsung-soc
In-Reply-To: <20190725144300.25014-1-k.konieczny@partner.samsung.com>

From: Marek Szyprowski <m.szyprowski@samsung.com>

Declare Exynos5422/5800 voltage ranges for opp points for big cpu core and
bus wcore and couple their voltage supllies as vdd_arm and vdd_int should
be in 300mV range.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
[k.konieczny: add missing patch description]
Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 arch/arm/boot/dts/exynos5420.dtsi             | 34 +++++++++----------
 arch/arm/boot/dts/exynos5422-odroid-core.dtsi |  4 +++
 arch/arm/boot/dts/exynos5800-peach-pi.dts     |  4 +++
 arch/arm/boot/dts/exynos5800.dtsi             | 32 ++++++++---------
 4 files changed, 41 insertions(+), 33 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index 5fb2326875dc..0cbf74750553 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -48,62 +48,62 @@
 			opp-shared;
 			opp-1800000000 {
 				opp-hz = /bits/ 64 <1800000000>;
-				opp-microvolt = <1250000>;
+				opp-microvolt = <1250000 1250000 1500000>;
 				clock-latency-ns = <140000>;
 			};
 			opp-1700000000 {
 				opp-hz = /bits/ 64 <1700000000>;
-				opp-microvolt = <1212500>;
+				opp-microvolt = <1212500 1212500 1500000>;
 				clock-latency-ns = <140000>;
 			};
 			opp-1600000000 {
 				opp-hz = /bits/ 64 <1600000000>;
-				opp-microvolt = <1175000>;
+				opp-microvolt = <1175000 1175000 1500000>;
 				clock-latency-ns = <140000>;
 			};
 			opp-1500000000 {
 				opp-hz = /bits/ 64 <1500000000>;
-				opp-microvolt = <1137500>;
+				opp-microvolt = <1137500 1137500 1500000>;
 				clock-latency-ns = <140000>;
 			};
 			opp-1400000000 {
 				opp-hz = /bits/ 64 <1400000000>;
-				opp-microvolt = <1112500>;
+				opp-microvolt = <1112500 1112500 1500000>;
 				clock-latency-ns = <140000>;
 			};
 			opp-1300000000 {
 				opp-hz = /bits/ 64 <1300000000>;
-				opp-microvolt = <1062500>;
+				opp-microvolt = <1062500 1062500 1500000>;
 				clock-latency-ns = <140000>;
 			};
 			opp-1200000000 {
 				opp-hz = /bits/ 64 <1200000000>;
-				opp-microvolt = <1037500>;
+				opp-microvolt = <1037500 1037500 1500000>;
 				clock-latency-ns = <140000>;
 			};
 			opp-1100000000 {
 				opp-hz = /bits/ 64 <1100000000>;
-				opp-microvolt = <1012500>;
+				opp-microvolt = <1012500 1012500 1500000>;
 				clock-latency-ns = <140000>;
 			};
 			opp-1000000000 {
 				opp-hz = /bits/ 64 <1000000000>;
-				opp-microvolt = < 987500>;
+				opp-microvolt = < 987500 987500 1500000>;
 				clock-latency-ns = <140000>;
 			};
 			opp-900000000 {
 				opp-hz = /bits/ 64 <900000000>;
-				opp-microvolt = < 962500>;
+				opp-microvolt = < 962500 962500 1500000>;
 				clock-latency-ns = <140000>;
 			};
 			opp-800000000 {
 				opp-hz = /bits/ 64 <800000000>;
-				opp-microvolt = < 937500>;
+				opp-microvolt = < 937500 937500 1500000>;
 				clock-latency-ns = <140000>;
 			};
 			opp-700000000 {
 				opp-hz = /bits/ 64 <700000000>;
-				opp-microvolt = < 912500>;
+				opp-microvolt = < 912500 912500 1500000>;
 				clock-latency-ns = <140000>;
 			};
 		};
@@ -1100,23 +1100,23 @@
 
 			opp00 {
 				opp-hz = /bits/ 64 <84000000>;
-				opp-microvolt = <925000>;
+				opp-microvolt = <925000 925000 1400000>;
 			};
 			opp01 {
 				opp-hz = /bits/ 64 <111000000>;
-				opp-microvolt = <950000>;
+				opp-microvolt = <950000 950000 1400000>;
 			};
 			opp02 {
 				opp-hz = /bits/ 64 <222000000>;
-				opp-microvolt = <950000>;
+				opp-microvolt = <950000 950000 1400000>;
 			};
 			opp03 {
 				opp-hz = /bits/ 64 <333000000>;
-				opp-microvolt = <950000>;
+				opp-microvolt = <950000 950000 1400000>;
 			};
 			opp04 {
 				opp-hz = /bits/ 64 <400000000>;
-				opp-microvolt = <987500>;
+				opp-microvolt = <987500 987500 1400000>;
 			};
 		};
 
diff --git a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
index 25d95de15c9b..65d094256b54 100644
--- a/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
+++ b/arch/arm/boot/dts/exynos5422-odroid-core.dtsi
@@ -428,6 +428,8 @@
 				regulator-max-microvolt = <1500000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-coupled-with = <&buck3_reg>;
+				regulator-coupled-max-spread = <300000>;
 			};
 
 			buck3_reg: BUCK3 {
@@ -436,6 +438,8 @@
 				regulator-max-microvolt = <1400000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-coupled-with = <&buck2_reg>;
+				regulator-coupled-max-spread = <300000>;
 			};
 
 			buck4_reg: BUCK4 {
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index e0f470fe54c8..5c1e965ed7e9 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -257,6 +257,8 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-coupled-with = <&buck3_reg>;
+				regulator-coupled-max-spread = <300000>;
 				regulator-state-mem {
 					regulator-off-in-suspend;
 				};
@@ -269,6 +271,8 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-coupled-with = <&buck2_reg>;
+				regulator-coupled-max-spread = <300000>;
 				regulator-state-mem {
 					regulator-off-in-suspend;
 				};
diff --git a/arch/arm/boot/dts/exynos5800.dtsi b/arch/arm/boot/dts/exynos5800.dtsi
index 57d3b319fd65..2a74735d161c 100644
--- a/arch/arm/boot/dts/exynos5800.dtsi
+++ b/arch/arm/boot/dts/exynos5800.dtsi
@@ -22,61 +22,61 @@
 
 &cluster_a15_opp_table {
 	opp-1700000000 {
-		opp-microvolt = <1250000>;
+		opp-microvolt = <1250000 1250000 1500000>;
 	};
 	opp-1600000000 {
-		opp-microvolt = <1250000>;
+		opp-microvolt = <1250000 1250000 1500000>;
 	};
 	opp-1500000000 {
-		opp-microvolt = <1100000>;
+		opp-microvolt = <1100000 1100000 1500000>;
 	};
 	opp-1400000000 {
-		opp-microvolt = <1100000>;
+		opp-microvolt = <1100000 1100000 1500000>;
 	};
 	opp-1300000000 {
-		opp-microvolt = <1100000>;
+		opp-microvolt = <1100000 1100000 1500000>;
 	};
 	opp-1200000000 {
-		opp-microvolt = <1000000>;
+		opp-microvolt = <1000000 1000000 1500000>;
 	};
 	opp-1100000000 {
-		opp-microvolt = <1000000>;
+		opp-microvolt = <1000000 1000000 1500000>;
 	};
 	opp-1000000000 {
-		opp-microvolt = <1000000>;
+		opp-microvolt = <1000000 1000000 1500000>;
 	};
 	opp-900000000 {
-		opp-microvolt = <1000000>;
+		opp-microvolt = <1000000 1000000 1500000>;
 	};
 	opp-800000000 {
-		opp-microvolt = <900000>;
+		opp-microvolt = <900000 900000 1500000>;
 	};
 	opp-700000000 {
-		opp-microvolt = <900000>;
+		opp-microvolt = <900000 900000 1500000>;
 	};
 	opp-600000000 {
 		opp-hz = /bits/ 64 <600000000>;
-		opp-microvolt = <900000>;
+		opp-microvolt = <900000 900000 1500000>;
 		clock-latency-ns = <140000>;
 	};
 	opp-500000000 {
 		opp-hz = /bits/ 64 <500000000>;
-		opp-microvolt = <900000>;
+		opp-microvolt = <900000 900000 1500000>;
 		clock-latency-ns = <140000>;
 	};
 	opp-400000000 {
 		opp-hz = /bits/ 64 <400000000>;
-		opp-microvolt = <900000>;
+		opp-microvolt = <900000 900000 1500000>;
 		clock-latency-ns = <140000>;
 	};
 	opp-300000000 {
 		opp-hz = /bits/ 64 <300000000>;
-		opp-microvolt = <900000>;
+		opp-microvolt = <900000 900000 1500000>;
 		clock-latency-ns = <140000>;
 	};
 	opp-200000000 {
 		opp-hz = /bits/ 64 <200000000>;
-		opp-microvolt = <900000>;
+		opp-microvolt = <900000 900000 1500000>;
 		clock-latency-ns = <140000>;
 	};
 };
-- 
2.22.0

^ permalink raw reply related

* [PATCH v4 2/5] devfreq: exynos-bus: convert to use dev_pm_opp_set_rate()
From: k.konieczny @ 2019-07-25 14:42 UTC (permalink / raw)
  To: k.konieczny
  Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
	Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
	MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
	Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
	linux-pm, linux-samsung-soc
In-Reply-To: <20190725144300.25014-1-k.konieczny@partner.samsung.com>

Reuse opp core code for setting bus clock and voltage. As a side
effect this allow usage of coupled regulators feature (required
for boards using Exynos5422/5800 SoCs) because dev_pm_opp_set_rate()
uses regulator_set_voltage_triplet() for setting regulator voltage
while the old code used regulator_set_voltage_tol() with fixed
tolerance. This patch also removes no longer needed parsing of DT
property "exynos,voltage-tolerance" (no Exynos devfreq DT node uses
it).

Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
---
Changes:
v4:
- remove unrelated changes, add newline before comment

---
 drivers/devfreq/exynos-bus.c | 108 +++++++++--------------------------
 1 file changed, 28 insertions(+), 80 deletions(-)

diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index f34fa26f00d0..ebb8f46312b6 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -25,7 +25,6 @@
 #include <linux/slab.h>
 
 #define DEFAULT_SATURATION_RATIO	40
-#define DEFAULT_VOLTAGE_TOLERANCE	2
 
 struct exynos_bus {
 	struct device *dev;
@@ -37,9 +36,8 @@ struct exynos_bus {
 
 	unsigned long curr_freq;
 
-	struct regulator *regulator;
+	struct opp_table *opp_table;
 	struct clk *clk;
-	unsigned int voltage_tolerance;
 	unsigned int ratio;
 };
 
@@ -99,56 +97,23 @@ static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 flags)
 {
 	struct exynos_bus *bus = dev_get_drvdata(dev);
 	struct dev_pm_opp *new_opp;
-	unsigned long old_freq, new_freq, new_volt, tol;
 	int ret = 0;
 
-	/* Get new opp-bus instance according to new bus clock */
+	/* Get correct frequency for bus. */
 	new_opp = devfreq_recommended_opp(dev, freq, flags);
 	if (IS_ERR(new_opp)) {
 		dev_err(dev, "failed to get recommended opp instance\n");
 		return PTR_ERR(new_opp);
 	}
 
-	new_freq = dev_pm_opp_get_freq(new_opp);
-	new_volt = dev_pm_opp_get_voltage(new_opp);
 	dev_pm_opp_put(new_opp);
 
-	old_freq = bus->curr_freq;
-
-	if (old_freq == new_freq)
-		return 0;
-	tol = new_volt * bus->voltage_tolerance / 100;
-
 	/* Change voltage and frequency according to new OPP level */
 	mutex_lock(&bus->lock);
+	ret = dev_pm_opp_set_rate(dev, *freq);
+	if (!ret)
+		bus->curr_freq = *freq;
 
-	if (old_freq < new_freq) {
-		ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
-		if (ret < 0) {
-			dev_err(bus->dev, "failed to set voltage\n");
-			goto out;
-		}
-	}
-
-	ret = clk_set_rate(bus->clk, new_freq);
-	if (ret < 0) {
-		dev_err(dev, "failed to change clock of bus\n");
-		clk_set_rate(bus->clk, old_freq);
-		goto out;
-	}
-
-	if (old_freq > new_freq) {
-		ret = regulator_set_voltage_tol(bus->regulator, new_volt, tol);
-		if (ret < 0) {
-			dev_err(bus->dev, "failed to set voltage\n");
-			goto out;
-		}
-	}
-	bus->curr_freq = new_freq;
-
-	dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
-			old_freq, new_freq, clk_get_rate(bus->clk));
-out:
 	mutex_unlock(&bus->lock);
 
 	return ret;
@@ -196,8 +161,10 @@ static void exynos_bus_exit(struct device *dev)
 
 	dev_pm_opp_of_remove_table(dev);
 	clk_disable_unprepare(bus->clk);
-	if (bus->regulator)
-		regulator_disable(bus->regulator);
+	if (bus->opp_table) {
+		dev_pm_opp_put_regulators(bus->opp_table);
+		bus->opp_table = NULL;
+	}
 }
 
 /*
@@ -208,39 +175,23 @@ static int exynos_bus_passive_target(struct device *dev, unsigned long *freq,
 {
 	struct exynos_bus *bus = dev_get_drvdata(dev);
 	struct dev_pm_opp *new_opp;
-	unsigned long old_freq, new_freq;
-	int ret = 0;
+	int ret;
 
-	/* Get new opp-bus instance according to new bus clock */
+	/* Get correct frequency for bus. */
 	new_opp = devfreq_recommended_opp(dev, freq, flags);
 	if (IS_ERR(new_opp)) {
 		dev_err(dev, "failed to get recommended opp instance\n");
 		return PTR_ERR(new_opp);
 	}
 
-	new_freq = dev_pm_opp_get_freq(new_opp);
 	dev_pm_opp_put(new_opp);
 
-	old_freq = bus->curr_freq;
-
-	if (old_freq == new_freq)
-		return 0;
-
 	/* Change the frequency according to new OPP level */
 	mutex_lock(&bus->lock);
+	ret = dev_pm_opp_set_rate(dev, *freq);
+	if (!ret)
+		bus->curr_freq = *freq;
 
-	ret = clk_set_rate(bus->clk, new_freq);
-	if (ret < 0) {
-		dev_err(dev, "failed to set the clock of bus\n");
-		goto out;
-	}
-
-	*freq = new_freq;
-	bus->curr_freq = new_freq;
-
-	dev_dbg(dev, "Set the frequency of bus (%luHz -> %luHz, %luHz)\n",
-			old_freq, new_freq, clk_get_rate(bus->clk));
-out:
 	mutex_unlock(&bus->lock);
 
 	return ret;
@@ -258,21 +209,19 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
 					struct exynos_bus *bus)
 {
 	struct device *dev = bus->dev;
+	struct opp_table *opp_table;
+	const char *vdd = "vdd";
 	int i, ret, count, size;
 
-	/* Get the regulator to provide each bus with the power */
-	bus->regulator = devm_regulator_get(dev, "vdd");
-	if (IS_ERR(bus->regulator)) {
-		dev_err(dev, "failed to get VDD regulator\n");
-		return PTR_ERR(bus->regulator);
-	}
-
-	ret = regulator_enable(bus->regulator);
-	if (ret < 0) {
-		dev_err(dev, "failed to enable VDD regulator\n");
+	opp_table = dev_pm_opp_set_regulators(dev, &vdd, 1);
+	if (IS_ERR(opp_table)) {
+		ret = PTR_ERR(opp_table);
+		dev_err(dev, "failed to set regulators %d\n", ret);
 		return ret;
 	}
 
+	bus->opp_table = opp_table;
+
 	/*
 	 * Get the devfreq-event devices to get the current utilization of
 	 * buses. This raw data will be used in devfreq ondemand governor.
@@ -313,14 +262,11 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
 	if (of_property_read_u32(np, "exynos,saturation-ratio", &bus->ratio))
 		bus->ratio = DEFAULT_SATURATION_RATIO;
 
-	if (of_property_read_u32(np, "exynos,voltage-tolerance",
-					&bus->voltage_tolerance))
-		bus->voltage_tolerance = DEFAULT_VOLTAGE_TOLERANCE;
-
 	return 0;
 
 err_regulator:
-	regulator_disable(bus->regulator);
+	dev_pm_opp_put_regulators(bus->opp_table);
+	bus->opp_table = NULL;
 
 	return ret;
 }
@@ -511,8 +457,10 @@ static int exynos_bus_probe(struct platform_device *pdev)
 	dev_pm_opp_of_remove_table(dev);
 	clk_disable_unprepare(bus->clk);
 err_reg:
-	if (!passive)
-		regulator_disable(bus->regulator);
+	if (!passive) {
+		dev_pm_opp_put_regulators(bus->opp_table);
+		bus->opp_table = NULL;
+	}
 
 	return ret;
 }
-- 
2.22.0

^ permalink raw reply related

* [PATCH v4 1/5] devfreq: exynos-bus: correct clock enable sequence
From: k.konieczny @ 2019-07-25 14:42 UTC (permalink / raw)
  To: k.konieczny
  Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
	Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
	MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
	Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
	linux-pm, linux-samsung-soc
In-Reply-To: <20190725144300.25014-1-k.konieczny@partner.samsung.com>

Regulators should be enabled before clocks to avoid h/w hang. This
require change in exynos_bus_probe() to move exynos_bus_parse_of()
after exynos_bus_parent_parse_of() and change in error handling.
Similar change is needed in exynos_bus_exit() where clock should be
disabled before regulators.

Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
---
Changes:
v4:
- move regulator disable after clock disable
- remove unrelated changes
- add disabling regulators in error path in exynos_bus_probe()

---
 drivers/devfreq/exynos-bus.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 486cc5b422f1..f34fa26f00d0 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -194,11 +194,10 @@ static void exynos_bus_exit(struct device *dev)
 	if (ret < 0)
 		dev_warn(dev, "failed to disable the devfreq-event devices\n");
 
-	if (bus->regulator)
-		regulator_disable(bus->regulator);
-
 	dev_pm_opp_of_remove_table(dev);
 	clk_disable_unprepare(bus->clk);
+	if (bus->regulator)
+		regulator_disable(bus->regulator);
 }
 
 /*
@@ -386,6 +385,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
 	struct exynos_bus *bus;
 	int ret, max_state;
 	unsigned long min_freq, max_freq;
+	bool passive = false;
 
 	if (!np) {
 		dev_err(dev, "failed to find devicetree node\n");
@@ -399,27 +399,27 @@ static int exynos_bus_probe(struct platform_device *pdev)
 	bus->dev = &pdev->dev;
 	platform_set_drvdata(pdev, bus);
 
-	/* Parse the device-tree to get the resource information */
-	ret = exynos_bus_parse_of(np, bus);
-	if (ret < 0)
-		return ret;
-
 	profile = devm_kzalloc(dev, sizeof(*profile), GFP_KERNEL);
-	if (!profile) {
-		ret = -ENOMEM;
-		goto err;
-	}
+	if (!profile)
+		return -ENOMEM;
 
 	node = of_parse_phandle(dev->of_node, "devfreq", 0);
 	if (node) {
 		of_node_put(node);
-		goto passive;
+		passive = true;
 	} else {
 		ret = exynos_bus_parent_parse_of(np, bus);
+		if (ret < 0)
+			return ret;
 	}
 
+	/* Parse the device-tree to get the resource information */
+	ret = exynos_bus_parse_of(np, bus);
 	if (ret < 0)
-		goto err;
+		goto err_reg;
+
+	if (passive)
+		goto passive;
 
 	/* Initialize the struct profile and governor data for parent device */
 	profile->polling_ms = 50;
@@ -510,6 +510,9 @@ static int exynos_bus_probe(struct platform_device *pdev)
 err:
 	dev_pm_opp_of_remove_table(dev);
 	clk_disable_unprepare(bus->clk);
+err_reg:
+	if (!passive)
+		regulator_disable(bus->regulator);
 
 	return ret;
 }
-- 
2.22.0

^ permalink raw reply related

* [PATCH v4 0/5] add coupled regulators for Exynos5422/5800
From: k.konieczny @ 2019-07-25 14:42 UTC (permalink / raw)
  To: k.konieczny
  Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
	Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
	MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
	Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
	linux-pm, linux-samsung-soc
In-Reply-To: <CGME20190725144324eucas1p211802237e336852ea3d1aa003e4d5d04@eucas1p2.samsung.com>

Hi,

The main purpose of this patch series is to add coupled regulators for
Exynos5422/5800 to keep constrain on voltage difference between vdd_arm
and vdd_int to be at most 300mV. In exynos-bus instead of using
regulator_set_voltage_tol() with default voltage tolerance it should be
used regulator_set_voltage_triplet() with volatege range, and this is
already present in opp/core.c code, so it can be reused. While at this,
move setting regulators into opp/core.

This patchset was tested on Odroid XU3.

The DTS coupled regulators patch depends on previous patches.

Changes:
v4:
- removed "opp: core: add regulators enable and disable" from patchset
  as it was applied by Viresh Kumar and changed cover letter
- fix patch "devfreq: exynos-bus: correct clock enable sequence" to
  correct order of enable/disable
- removed unrelated changes in "devfreq: exynos-bus: convert to use
  dev_pm_opp_set_rate()"
- added new patch "devfreq: exynos-bus: remove exynos_bus_passive_target()"
  as suggested by Chanwoo Choi
v3:
- added new exynos-bus patch to correct clock and regulator enabling
  and disabling sequence as suggested by Chanwoo Choi
- corrected error path in enable and improved commit message in opp/core
- improve comment in devfreq/exynos-bus.c before devfreq_recommended_opp()
- change cover letter as there is new patch
- added note before Signed-off-by in 4th patch
v2:
- improve regulators enable/disable code in opp/core as suggested by
  Viresh Kumar
- add new patch for remove unused dt-bindings as suggested by Krzysztof
  Kozlowski

Kamil Konieczny (4):
  devfreq: exynos-bus: correct clock enable sequence
  devfreq: exynos-bus: convert to use dev_pm_opp_set_rate()
  dt-bindings: devfreq: exynos-bus: remove unused property
  devfreq: exynos-bus: remove exynos_bus_passive_target()

Marek Szyprowski (1):
  ARM: dts: exynos: add initial data for coupled regulators for
    Exynos5422/5800

 .../bindings/devfreq/exynos-bus.txt           |   2 -
 arch/arm/boot/dts/exynos5420.dtsi             |  34 ++--
 arch/arm/boot/dts/exynos5422-odroid-core.dtsi |   4 +
 arch/arm/boot/dts/exynos5800-peach-pi.dts     |   4 +
 arch/arm/boot/dts/exynos5800.dtsi             |  32 ++--
 drivers/devfreq/exynos-bus.c                  | 153 +++++-------------
 6 files changed, 78 insertions(+), 151 deletions(-)

-- 
2.22.0

^ permalink raw reply

* Re: [RFC PATCH v2 0/4] Input: mpr121-polled: Add polled driver for MPR121
From: Dmitry Torokhov @ 2019-07-25 14:40 UTC (permalink / raw)
  To: Michal Vokáč
  Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Fabio Estevam,
	linux-input, devicetree, linux-kernel, Pengutronix Kernel Team
In-Reply-To: <ac436c3c-fa89-f777-85b2-f38adf842e10@ysoft.com>

On Thu, Jul 25, 2019 at 02:58:02PM +0200, Michal Vokáč wrote:
> On 25. 07. 19 10:57, Dmitry Torokhov wrote:
> > Hi Michal,
> > 
> > On Tue, May 21, 2019 at 08:51:17AM +0200, Michal Vokáč wrote:
> > > On 21. 05. 19 7:37, Dmitry Torokhov wrote:
> > > > Hi Michal,
> > > > 
> > > > On Fri, May 17, 2019 at 03:12:49PM +0200, Michal Vokáč wrote:
> > > > > Hi,
> > > > > 
> > > > > I have to deal with a situation where we have a custom i.MX6 based
> > > > > platform in production that uses the MPR121 touchkey controller.
> > > > > Unfortunately the chip is connected using only the I2C interface.
> > > > > The interrupt line is not used. Back in 2015 (Linux v3.14), my
> > > > > colleague modded the existing mpr121_touchkey.c driver to use polling
> > > > > instead of interrupt.
> > > > > 
> > > > > For quite some time yet I am in a process of updating the product from
> > > > > the ancient Freescale v3.14 kernel to the latest mainline and pushing
> > > > > any needed changes upstream. The DT files for our imx6dl-yapp4 platform
> > > > > already made it into v5.1-rc.
> > > > > 
> > > > > I rebased and updated our mpr121 patch to the latest mainline.
> > > > > It is created as a separate driver, similarly to gpio_keys_polled.
> > > > > 
> > > > > The I2C device is quite susceptible to ESD. An ESD test quite often
> > > > > causes reset of the chip or some register randomly changes its value.
> > > > > The [PATCH 3/4] adds a write-through register cache. With the cache
> > > > > this state can be detected and the device can be re-initialied.
> > > > > 
> > > > > The main question is: Is there any chance that such a polled driver
> > > > > could be accepted? Is it correct to implement it as a separate driver
> > > > > or should it be done as an option in the existing driver? I can not
> > > > > really imagine how I would do that though..
> > > > > 
> > > > > There are also certain worries that the MPR121 chip may no longer be
> > > > > available in nonspecifically distant future. In case of EOL I will need
> > > > > to add a polled driver for an other touchkey chip. May it be already
> > > > > in mainline or a completely new one.
> > > > 
> > > > I think that my addition of input_polled_dev was ultimately a wrong
> > > > thing to do. I am looking into enabling polling mode for regular input
> > > > devices as we then can enable polling mode in existing drivers.
> > > 
> > > OK, that sounds good. Especially when one needs to switch from one chip
> > > to another that is already in tree, the need for a whole new polling
> > > driver is eliminated.
> > 
> > Could you please try the patch below and see if it works for your use
> > case? Note that I have not tried running it, but it compiles so it must
> > be good ;)
> 
> Hi Dmitry,
> Thank you very much for the patch!
> I gave it a shot and it seems you forgot to add the input-poller.h file
> to the patch.. it does not compile on my side :(

Oops ;) Please see the updated patch below.

> 
> > Input: add support for polling to input devices
> > 
> > From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > 
> > Separating "normal" and "polled" input devices was a mistake, as often we want
> > to allow the very same device work on both interrupt-driven and polled mode,
> > depending on the board on which the device is used.
> > 
> > This introduces new APIs:
> > 
> > - input_setup_polling
> > - input_set_poll_interval
> > - input_set_min_poll_interval
> > - input_set_max_poll_interval
> > 
> > These new APIs allow switching an input device into polled mode with sysfs
> > attributes matching drivers using input_polled_dev APIs that will be eventually
> > removed.
> 
> After reading this I am not really sure what else needs to be done
> to test/use the poller. I suspect I need to modify the input device
> driver (mpr121_touchkey.c in my case) like this:
> 
> If the interrupt gpio is not provided in DT, the device driver probe
> function should:
>  - not request the threaded interrupt
>  - call input_setup_polling and provide it with poll_fn
>    Can the mpr_touchkey_interrupt function be used as is for this
>    purpose? The only problem I see is it returns IRQ_HANDLED.

I'd factor out code suitable for polling from mpr_touchkey_interrupt()
and then do

static irqreturn_t mpr_touchkey_interrupt(...)
{
	mpr_touchkey_report(...);
	return IRQ_HANDLED;
}

> 
>  - set the poll interval + min/max interval
>  - register the input device as usual (input_register_device)

Yes.

>  - What about the device_init_wakeup? It does not make sense for
>    polling I think.

We can either trust DT not to have the property for polled case, or add
more checks. I think we should simply trust DT. Even if it is wrong it
will not cause any issues.

> 
> Just nitpicking - I also ran checkpatch.pl on the patch and it spits
> out some warnings. I am not sure whether some of those should not
> be fixed.

I think in this particular case checkpatch is full of it.

Thanks.


Input: add support for polling to input devices

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Separating "normal" and "polled" input devices was a mistake, as often we want
to allow the very same device work on both interrupt-driven and polled mode,
depending on the board on which the device is used.

This introduces new APIs:

- input_setup_polling
- input_set_poll_interval
- input_set_min_poll_interval
- input_set_max_poll_interval

These new APIs allow switching an input device into polled mode with sysfs
attributes matching drivers using input_polled_dev APIs that will be eventually
removed.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/Makefile       |    2 
 drivers/input/input-poller.c |  207 ++++++++++++++++++++++++++++++++++++++++++
 drivers/input/input-poller.h |   18 ++++
 drivers/input/input.c        |   36 ++++++-
 include/linux/input.h        |   10 ++
 5 files changed, 265 insertions(+), 8 deletions(-)
 create mode 100644 drivers/input/input-poller.c
 create mode 100644 drivers/input/input-poller.h

diff --git a/drivers/input/Makefile b/drivers/input/Makefile
index 40de6a7be641..e35650930371 100644
--- a/drivers/input/Makefile
+++ b/drivers/input/Makefile
@@ -6,7 +6,7 @@
 # Each configuration option enables a list of files.
 
 obj-$(CONFIG_INPUT)		+= input-core.o
-input-core-y := input.o input-compat.o input-mt.o ff-core.o
+input-core-y := input.o input-compat.o input-mt.o input-poller.o ff-core.o
 
 obj-$(CONFIG_INPUT_FF_MEMLESS)	+= ff-memless.o
 obj-$(CONFIG_INPUT_POLLDEV)	+= input-polldev.o
diff --git a/drivers/input/input-poller.c b/drivers/input/input-poller.c
new file mode 100644
index 000000000000..008d6f362d60
--- /dev/null
+++ b/drivers/input/input-poller.c
@@ -0,0 +1,207 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Support for polling mode for input devices.
+ */
+
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/jiffies.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+#include "input-poller.h"
+
+struct input_dev_poller {
+	void (*poll)(struct input_dev *dev);
+
+	unsigned int poll_interval; /* msec */
+	unsigned int poll_interval_max; /* msec */
+	unsigned int poll_interval_min; /* msec */
+
+	struct input_dev *input;
+	struct delayed_work work;
+};
+
+static void input_dev_poller_queue_work(struct input_dev_poller *poller)
+{
+	unsigned long delay;
+
+	delay = msecs_to_jiffies(poller->poll_interval);
+	if (delay >= HZ)
+		delay = round_jiffies_relative(delay);
+
+	queue_delayed_work(system_freezable_wq, &poller->work, delay);
+}
+
+static void input_dev_poller_work(struct work_struct *work)
+{
+	struct input_dev_poller *poller =
+		container_of(work, struct input_dev_poller, work.work);
+
+	poller->poll(poller->input);
+	input_dev_poller_queue_work(poller);
+}
+
+void input_dev_poller_finalize(struct input_dev_poller *poller)
+{
+	if (!poller->poll_interval)
+		poller->poll_interval = 500;
+	if (!poller->poll_interval_max)
+		poller->poll_interval_max = poller->poll_interval;
+}
+
+void input_dev_poller_start(struct input_dev_poller *poller)
+{
+	/* Only start polling if polling is enabled */
+	if (poller->poll_interval > 0) {
+		poller->poll(poller->input);
+		input_dev_poller_queue_work(poller);
+	}
+}
+
+void input_dev_poller_stop(struct input_dev_poller *poller)
+{
+	cancel_delayed_work_sync(&poller->work);
+}
+
+int input_setup_polling(struct input_dev *dev,
+			void (*poll_fn)(struct input_dev *dev))
+{
+	struct input_dev_poller *poller;
+
+	poller = kzalloc(sizeof(*poller), GFP_KERNEL);
+	if (!poller) {
+		dev_err(dev->dev.parent ?: &dev->dev,
+			"%s: unable to allocate poller structure\n", __func__);
+		return -ENOMEM;
+	}
+
+	INIT_DELAYED_WORK(&poller->work, input_dev_poller_work);
+	poller->poll = poll_fn;
+
+	dev->poller = poller;
+	return 0;
+}
+EXPORT_SYMBOL(input_setup_polling);
+
+static bool input_dev_ensure_poller(struct input_dev *dev)
+{
+	if (!dev->poller) {
+		dev_err(dev->dev.parent ?: &dev->dev,
+			"poller structure has not been set up\n");
+		return false;
+	}
+
+	return true;
+}
+
+void input_set_poll_interval(struct input_dev *dev, unsigned int interval)
+{
+	if (input_dev_ensure_poller(dev))
+		dev->poller->poll_interval = interval;
+}
+EXPORT_SYMBOL(input_set_poll_interval);
+
+void input_set_min_poll_interval(struct input_dev *dev, unsigned int interval)
+{
+	if (input_dev_ensure_poller(dev))
+		dev->poller->poll_interval_min = interval;
+}
+EXPORT_SYMBOL(input_set_min_poll_interval);
+
+void input_set_max_poll_interval(struct input_dev *dev, unsigned int interval)
+{
+	if (input_dev_ensure_poller(dev))
+		dev->poller->poll_interval_max = interval;
+}
+EXPORT_SYMBOL(input_set_max_poll_interval);
+
+/* SYSFS interface */
+
+static ssize_t input_dev_get_poll_interval(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct input_dev *input = to_input_dev(dev);
+
+	return sprintf(buf, "%d\n", input->poller->poll_interval);
+}
+
+static ssize_t input_dev_set_poll_interval(struct device *dev,
+					   struct device_attribute *attr,
+					   const char *buf, size_t count)
+{
+	struct input_dev *input = to_input_dev(dev);
+	struct input_dev_poller *poller = input->poller;
+	unsigned int interval;
+	int err;
+
+	err = kstrtouint(buf, 0, &interval);
+	if (err)
+		return err;
+
+	if (interval < poller->poll_interval_min)
+		return -EINVAL;
+
+	if (interval > poller->poll_interval_max)
+		return -EINVAL;
+
+	mutex_lock(&input->mutex);
+
+	poller->poll_interval = interval;
+
+	if (input->users) {
+		cancel_delayed_work_sync(&poller->work);
+		if (poller->poll_interval > 0)
+			input_dev_poller_queue_work(poller);
+	}
+
+	mutex_unlock(&input->mutex);
+
+	return count;
+}
+
+static DEVICE_ATTR(poll, S_IRUGO | S_IWUSR,
+		   input_dev_get_poll_interval, input_dev_set_poll_interval);
+
+static ssize_t input_dev_get_poll_max(struct device *dev,
+				      struct device_attribute *attr, char *buf)
+{
+	struct input_dev *input = to_input_dev(dev);
+
+	return sprintf(buf, "%d\n", input->poller->poll_interval_max);
+}
+
+static DEVICE_ATTR(max, S_IRUGO, input_dev_get_poll_max, NULL);
+
+static ssize_t input_dev_get_poll_min(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	struct input_dev *input = to_input_dev(dev);
+
+	return sprintf(buf, "%d\n", input->poller->poll_interval_min);
+}
+
+static DEVICE_ATTR(min, S_IRUGO, input_dev_get_poll_min, NULL);
+
+static umode_t input_poller_attrs_visible(struct kobject *kobj,
+					  struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct input_dev *input = to_input_dev(dev);
+
+	return input->poller ? attr->mode : 0;
+}
+
+static struct attribute *input_poller_attrs[] = {
+	&dev_attr_poll.attr,
+	&dev_attr_max.attr,
+	&dev_attr_min.attr,
+	NULL
+};
+
+struct attribute_group input_poller_attribute_group = {
+	.is_visible	= input_poller_attrs_visible,
+	.attrs		= input_poller_attrs,
+};
diff --git a/drivers/input/input-poller.h b/drivers/input/input-poller.h
new file mode 100644
index 000000000000..e3fca0be1d32
--- /dev/null
+++ b/drivers/input/input-poller.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _INPUT_POLLER_H
+#define _INPUT_POLLER_H
+
+/*
+ * Support for polling mode for input devices.
+ */
+#include <linux/sysfs.h>
+
+struct input_dev_poller;
+
+void input_dev_poller_finalize(struct input_dev_poller *poller);
+void input_dev_poller_start(struct input_dev_poller *poller);
+void input_dev_poller_stop(struct input_dev_poller *poller);
+
+extern struct attribute_group input_poller_attribute_group;
+
+#endif /* _INPUT_POLLER_H */
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 7494a0dede79..c08aa3596144 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -24,6 +24,7 @@
 #include <linux/mutex.h>
 #include <linux/rcupdate.h>
 #include "input-compat.h"
+#include "input-poller.h"
 
 MODULE_AUTHOR("Vojtech Pavlik <vojtech@suse.cz>");
 MODULE_DESCRIPTION("Input core");
@@ -603,20 +604,31 @@ int input_open_device(struct input_handle *handle)
 
 	handle->open++;
 
-	if (!dev->users++ && dev->open)
-		retval = dev->open(dev);
+	if (dev->users++) {
+		/*
+		 * Device is already opened, so we can exit immediately and
+		 * report success.
+		 */
+		goto out;
+	}
 
-	if (retval) {
-		dev->users--;
-		if (!--handle->open) {
+	if (dev->open) {
+		retval = dev->open(dev);
+		if (retval) {
+			dev->users--;
+			handle->open--;
 			/*
 			 * Make sure we are not delivering any more events
 			 * through this handle
 			 */
 			synchronize_rcu();
+			goto out;
 		}
 	}
 
+	if (dev->poller)
+		input_dev_poller_start(dev->poller);
+
  out:
 	mutex_unlock(&dev->mutex);
 	return retval;
@@ -655,8 +667,13 @@ void input_close_device(struct input_handle *handle)
 
 	__input_release_device(handle);
 
-	if (!--dev->users && dev->close)
-		dev->close(dev);
+	if (!--dev->users) {
+		if (dev->poller)
+			input_dev_poller_stop(dev->poller);
+
+		if (dev->close)
+			dev->close(dev);
+	}
 
 	if (!--handle->open) {
 		/*
@@ -1502,6 +1519,7 @@ static const struct attribute_group *input_dev_attr_groups[] = {
 	&input_dev_attr_group,
 	&input_dev_id_attr_group,
 	&input_dev_caps_attr_group,
+	&input_poller_attribute_group,
 	NULL
 };
 
@@ -1511,6 +1529,7 @@ static void input_dev_release(struct device *device)
 
 	input_ff_destroy(dev);
 	input_mt_destroy_slots(dev);
+	kfree(dev->poller);
 	kfree(dev->absinfo);
 	kfree(dev->vals);
 	kfree(dev);
@@ -2175,6 +2194,9 @@ int input_register_device(struct input_dev *dev)
 	if (!dev->setkeycode)
 		dev->setkeycode = input_default_setkeycode;
 
+	if (dev->poller)
+		input_dev_poller_finalize(dev->poller);
+
 	error = device_add(&dev->dev);
 	if (error)
 		goto err_free_vals;
diff --git a/include/linux/input.h b/include/linux/input.h
index e95a439d8bd5..10346211ff15 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -21,6 +21,8 @@
 #include <linux/timer.h>
 #include <linux/mod_devicetable.h>
 
+struct input_dev_poller;
+
 /**
  * struct input_value - input value representation
  * @type: type of value (EV_KEY, EV_ABS, etc)
@@ -156,6 +158,8 @@ struct input_dev {
 
 	struct ff_device *ff;
 
+	struct input_dev_poller *poller;
+
 	unsigned int repeat_key;
 	struct timer_list timer;
 
@@ -372,6 +376,12 @@ void input_unregister_device(struct input_dev *);
 
 void input_reset_device(struct input_dev *);
 
+int input_setup_polling(struct input_dev *dev,
+			void (*poll_fn)(struct input_dev *dev));
+void input_set_poll_interval(struct input_dev *dev, unsigned int interval);
+void input_set_min_poll_interval(struct input_dev *dev, unsigned int interval);
+void input_set_max_poll_interval(struct input_dev *dev, unsigned int interval);
+
 int __must_check input_register_handler(struct input_handler *);
 void input_unregister_handler(struct input_handler *);
 


-- 
Dmitry

^ permalink raw reply related

* Re: [PATCH 1/2] bus: imx-weim: optionally enable burst clock mode
From: Sven Van Asbroeck @ 2019-07-25 14:30 UTC (permalink / raw)
  To: Shawn Guo
  Cc: NXP Linux Team, Kees Cook, Linux Kernel Mailing List,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Mark Rutland, Sascha Hauer, devicetree, Fabio Estevam,
	Pengutronix Kernel Team, Arnd Bergmann
In-Reply-To: <20190712204316.16783-1-TheSven73@gmail.com>

On Fri, Jul 12, 2019 at 4:43 PM Sven Van Asbroeck <thesven73@gmail.com> wrote:
>
> To enable burst clock mode, add the fsl,burst-clk-enable
> property to the weim bus's devicetree node.
>

Any feedback on this patch, positive or negative?

^ permalink raw reply

* Re: [PATCH v5 2/4] dt-bindings: thermal: nvme: Add binding documentation
From: Akinobu Mita @ 2019-07-25 14:24 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-nvme, linux-pm, open list:OPEN FIRMWARE AND..., Zhang Rui,
	Eduardo Valentin, Daniel Lezcano, Keith Busch, Jens Axboe,
	Christoph Hellwig, Sagi Grimberg, Minwoo Im, Kenneth Heitke,
	Chaitanya Kulkarni
In-Reply-To: <20190722221645.GA32515@bogus>

2019年7月23日(火) 7:16 Rob Herring <robh@kernel.org>:
>
> On Mon, Jul 01, 2019 at 11:12:32PM +0900, Akinobu Mita wrote:
> > Add thermal binding documentation for NVMe temperature sensor.
> >
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Zhang Rui <rui.zhang@intel.com>
> > Cc: Eduardo Valentin <edubezval@gmail.com>
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: Keith Busch <kbusch@kernel.org>
> > Cc: Jens Axboe <axboe@fb.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Sagi Grimberg <sagi@grimberg.me>
> > Cc: Minwoo Im <minwoo.im.dev@gmail.com>
> > Cc: Kenneth Heitke <kenneth.heitke@intel.com>
> > Cc: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> > ---
> > * v5
> > - New patch
> >
> >  Documentation/devicetree/bindings/thermal/nvme.txt | 56 ++++++++++++++++++++++
> >  1 file changed, 56 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/thermal/nvme.txt
> >
> > diff --git a/Documentation/devicetree/bindings/thermal/nvme.txt b/Documentation/devicetree/bindings/thermal/nvme.txt
> > new file mode 100644
> > index 0000000..60b90de
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/thermal/nvme.txt
> > @@ -0,0 +1,56 @@
> > +Binding for NVMe temperature sensor
> > +
> > +An NVMe controller reports up to nine temperature values in the SMART / Health
> > +log.
> > +
> > +Required properties:
> > +- reg: A five-cell address encoded as (phys.hi phys.mid phys.lo size.hi
> > +  size.lo). phys.hi should contain the device's BDF (Bus/Device/Function)
> > +  as 0b00000000 bbbbbbbb dddddfff 00000000. The other cells should be zero.
> > +  See also Documentation/devicetree/bindings/pci/pci.txt
> > +
> > +- #thermal-sensor-cells: Must be 1. See ./thermal.txt for a description.
> > +  In the thermal-sensors property, the sensor ID 0 for composite temperature,
> > +  1 through 8 for NVMe temperature sensor N.
> > +
> > +Example:
> > +
> > +&pcie0 {
> > +     ...
> > +     nvme: nvme@0,0 {
> > +             reg = <0x0000 0 0 0 0>;
> > +             #address-cells = <3>;
> > +             #size-cells = <2>;
> > +
> > +             nvmetemp: nvmetemp {
> > +                     reg = <0x0000 0 0 0 0>; /* DEVFN = 0x00 (0:0) */
>
> I'm not sure this is really valid PCI addressing as the parent has the
> same address.
>
> > +                     #thermal-sensor-cells = <1>;
>
> Can't you just put this in the parent? Is this really a separate
> addressable device from the parent?

How about this?

&pcie0 {
...
        pci-bridge@0 {
                reg = <0x00000 0 0 0 0>;
                #address-cells = <3>;
                #size-cells = <2>;

                nvme: nvme@0,0 {
                        reg = <0x10000 0 0 0 0>;
                        #thermal-sensor-cells = <1>;
                };
        };
};

and

&thermal_zones {
...
thermal-sensors = <&nvme 0>;
};

I tested this with the RockPro64 and edited
arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts.

$ lspci
00:00.0 PCI bridge: Fuzhou Rockchip Electronics Co., Ltd Device 0100
01:00.0 Non-Volatile memory controller: Micron/Crucial Technology
Device 2263 (rev 03)

$ lspci -tv
-[0000:00]---00.0-[01]----00.0  Micron/Crucial Technology Device 2263

^ permalink raw reply

* Re: [RFCv3 1/3] dt-bindings: devfreq: Add initial bindings for i.MX
From: Chanwoo Choi @ 2019-07-25 14:23 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: MyungJoo Ham, Kyungmin Park, Will Deacon, Stephen Boyd,
	Michael Turquette, Jacky Bai, Anson Huang, Abel Vesa,
	Dong Aisheng, Viresh Kumar, Georgi Djakov, Alexandre Bailon,
	Chanwoo Choi, Mark Rutland, Frank Li, Rob Herring, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, NXP
In-Reply-To: <93df6e7d81a404a43af684e2f96bdb6561ed87fe.1563971855.git.leonard.crestez@nxp.com>

Hi,

2019년 7월 24일 (수) 오후 10:36, Leonard Crestez <leonard.crestez@nxp.com>님이 작성:
>
> Add initial dt bindings for the interconnects inside i.MX chips.
> Multiple external IPs are involved but SOC integration means the
> software controllable interfaces are very similar.
>
> This is initially only for imx8mm but add an "fsl,imx-bus" fallback
> similar to exynos-bus.
>
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  .../devicetree/bindings/devfreq/imx.yaml      | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/devfreq/imx.yaml
>
> diff --git a/Documentation/devicetree/bindings/devfreq/imx.yaml b/Documentation/devicetree/bindings/devfreq/imx.yaml
> new file mode 100644
> index 000000000000..87f90cddfd29
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/devfreq/imx.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/devfreq/imx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic i.MX bus frequency device
> +
> +maintainers:
> +  - Leonard Crestez <leonard.crestez@nxp.com>
> +
> +description: |
> +  The i.MX SoC family has multiple buses for which clock frequency (and sometimes
> +  voltage) can be adjusted.
> +
> +  Some of those buses expose register areas mentioned in the memory maps as GPV
> +  ("Global Programmers View") but not all. Access to this area might be denied for
> +  normal world.
> +
> +  The buses are based on externally licensed IPs such as ARM NIC-301 and Arteris
> +  FlexNOC but DT bindings are specific to the integration of these bus
> +  interconnect IPs into imx SOCs.
> +
> +properties:
> +  reg:
> +    maxItems: 1
> +    description: GPV area
> +
> +  compatible:
> +    contains:
> +      enum:
> +       - fsl,imx8m-noc
> +       - fsl,imx8m-nic
> +       - fsl,imx8m-ddrc
> +
> +  clocks:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - clocks
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx8mm-clock.h>
> +    ddrc: dram-controller@3d400000 {
> +            compatible = "fsl,imx8mm-ddrc";

s/imx8mm/imx8m

> +            reg = <0x3d400000 0x400000>;
> +            clocks = <&clk IMX8MM_CLK_DRAM>;
> +            operating-points-v2 = <&ddrc_opp_table>;
> +    };
> +
> +  - |
> +    noc: noc@32700000 {
> +            compatible = "fsl,imx8mm-noc";

s/imx8mm/imx8m

> +            reg = <0x32700000 0x100000>;
> +            clocks = <&clk IMX8MM_CLK_NOC>;
> +            operating-points-v2 = <&noc_opp_table>;
> +    };
> --
> 2.17.1
>


-- 
Best Regards,
Chanwoo Choi

^ permalink raw reply

* Re: [PATCH v2 1/2] dt-bindings: arm: imx: add imx8mq nitrogen support
From: Fabio Estevam @ 2019-07-25 14:11 UTC (permalink / raw)
  To: Dafna Hirschfeld
  Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Sascha Hauer,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Ezequiel Garcia, kernel, Gary Bisson, Troy Kisky
In-Reply-To: <20190725121452.16607-2-dafna.hirschfeld@collabora.com>

Hi Dafna,

On Thu, Jul 25, 2019 at 10:56 AM Dafna Hirschfeld
<dafna.hirschfeld@collabora.com> wrote:
>
> From: Gary Bisson <gary.bisson@boundarydevices.com>
>
> i.MX 8Quad is a quad (4x) Cortex-A53 processor with powerful
> graphic and multimedia features.
> This patch adds Nitrogen8M board support.
>
> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> [Dafna: porting vendor's code to mainline]
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
>  Documentation/devicetree/bindings/arm/fsl.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> index 7294ac36f4c0..728c41ef83bb 100644
> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> @@ -227,6 +227,12 @@ properties:
>                - fsl,imx8qxp-mek           # i.MX8QXP MEK Board
>            - const: fsl,imx8qxp
>
> +      - description: i.MX8MQ based Boards

This line is already present in latest code as we already have some
i.MX8MQ boards listed.

Please rebase it against linux-next.

^ permalink raw reply

* Re: [PATCH v3 0/7] drivers: Add generic device lookup helpers
From: Greg KH @ 2019-07-25 14:10 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: Andrew Lunn, Alexander Aring, Heikki Krogerus, Jacek Anaszewski,
	rafael, Alexander Shishkin, linux-fpga, Heiko Carstens,
	Alexandre Belloni, dri-devel, Liam Girdwood, Elie Morisse,
	Srinivas Kandagatla, linux-i2c, Pavel Machek, Shyam Sundar S K,
	Frank Rowand, linux-leds, linux-rtc, Maxime Ripard,
	Florian Fainelli, linux-acpi, Jason Gunthorpe,
	Lee Jones <lee.jo>
In-Reply-To: <20190723221838.12024-1-suzuki.poulose@arm.com>

On Tue, Jul 23, 2019 at 11:18:31PM +0100, Suzuki K Poulose wrote:
> We have device iterators to find a particular device matching a criteria
> for a given bus/class/driver. i.e, {bus,class,driver}_find_device()
> APIs. The matching criteria is a function pointer for the APIs. Often the
> lookup is based on a generic property of a device (e.g, name, fwnode, of node
> pointer or device type) rather than a driver specific information. However, each
> driver writes up its own "match" function, spilling the similar match functions
> all over the driver subsystems.
> 
> This series adds device match helpers by generic device properties of a device.
> Using these generic match functions, introduce wrappers to the existing
>  *_find_device() helpers and converts the existing users to use the new wrappers.
>  i.e,
> 	{bus/class/driver}_find_device_by_name()
> 	{bus/class/driver}_find_device_by_fwnode()
> 	{bus/class/driver}_find_device_by_devt()
> 	{bus/class/driver}_find_device_by_acpi_dev()
> 
> Also adds a wrapper for finding a platform device by driver, to avoid
> spilling the platform_bus internals in the individual drivers. Applies
> on 5.3-rc1.
> 
>    [0] git://linux-arm.org/linux-skp.git driver-cleanup/v3
> RFC[1] https://marc.info/?i=1559577023-558-1-git-send-email-suzuki.poulose@arm.com
> V1 [2] https://marc.info/?i=1559747630-28065-1-git-send-email-suzuki.poulose@arm.com
> V2 [3] https://marc.info/?i=1560534863-15115-1-git-send-email-suzuki.poulose@arm.com
> 
> Changes since v2:
>  - Merge the device match helper introduction patch with the variants
>    of the helpers, for better review.
>  - Use platform_find_device_by_driver for mcde drm driver.

This looks good to me at first glance, thanks for doing this work.

I'll wait a few days to collect acks from various subsystem maintainers
before taking the series in my tree.

thanks,

greg k-h
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH 2/2] arm64: dts: imx: Add i.mx8mq nitrogen8m basic dts support
From: Fabio Estevam @ 2019-07-25 14:07 UTC (permalink / raw)
  To: Dafna Hirschfeld
  Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Sascha Hauer,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Ezequiel Garcia, kernel, Gary Bisson
In-Reply-To: <20190724171747.26076-1-dafna.hirschfeld@collabora.com>

Hi Dafna,

On Wed, Jul 24, 2019 at 2:51 PM Dafna Hirschfeld
<dafna.hirschfeld@collabora.com> wrote:
>
> From: dafna <dafna.hirschfeld@collabora.com>
>
> Add basic dts support for i.MX8MQ NITROGEN8M.
>
> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>

Your From and Signed-off-by tags do not match.

> ---
>  arch/arm64/boot/dts/freescale/Makefile             |   1 +
>  .../arm64/boot/dts/freescale/imx8mq-nitrogen8m.dts | 411 +++++++++++++++++++++
>  2 files changed, 412 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mq-nitrogen8m.dts
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index c043aca66572..54a5c18c5c30 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -26,3 +26,4 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-zii-ultra-rmb3.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-zii-ultra-zest.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mq-nitrogen8m.dtb

Please keep it in alphabetical order.

> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-nitrogen8m.dts b/arch/arm64/boot/dts/freescale/imx8mq-nitrogen8m.dts
> new file mode 100644
> index 000000000000..cfd4915d2916
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-nitrogen8m.dts

Isn't the 8m in the end redundant? What about just naming it
imx8mq-nitrogen.dts instead?

> +&a53_opp_table {
> +               opp-1500000000 {
> +                       opp-hz = /bits/ 64 <1500000000>;
> +                       opp-microvolt = <1000000>;
> +               };
> +
> +               opp-1000000000 {
> +                       opp-hz = /bits/ 64 <1000000000>;
> +                       opp-microvolt = <900000>;
> +               };

Couldn't this be dropped and just use the operational points defined
at imx8mq.dtsi?

> +};
> +
> +&iomuxc {

We usually prefer to place iomux as the last node.

> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pinctrl_hog>;

You could place the "pinctrl_hog: hoggrp {" here

> +&i2c1 {
> +       clock-frequency = <400000>;
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pinctrl_i2c1>;
> +       status = "okay";
> +
> +       i2cmux@70 {
> +               compatible = "pca9546";

You missed the manufacturer string: "nxp,pca9546"

> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pinctrl_i2c1_pca9546>;
> +               reg = <0x70>;
> +               reset-gpios = <&gpio1 8 GPIO_ACTIVE_LOW>;
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               i2c1a: i2c1@0 {
> +                       reg = <0>;
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       reg_arm_dram: fan53555@60 {

Node names should be generic:

regulator@60

> +                               compatible = "fcs,fan53555";
> +                               pinctrl-names = "default";
> +                               pinctrl-0 = <&pinctrl_reg_arm_dram>;
> +                               reg = <0x60>;
> +                               regulator-min-microvolt =  <900000>;
> +                               regulator-max-microvolt = <1000000>;
> +                               regulator-always-on;
> +                               vsel-gpios = <&gpio3 24 GPIO_ACTIVE_HIGH>;
> +                       };
> +               };
> +
> +               i2c1b: i2c1@1 {
> +                       reg = <1>;
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       reg_dram_1p1v: fan53555@60 {

regulator@60

> +                               compatible = "fcs,fan53555";
> +                               pinctrl-names = "default";
> +                               pinctrl-0 = <&pinctrl_reg_dram_1p1v>;
> +                               reg = <0x60>;
> +                               regulator-min-microvolt = <1100000>;
> +                               regulator-max-microvolt = <1100000>;
> +                               regulator-always-on;
> +                               vsel-gpios = <&gpio2 11 GPIO_ACTIVE_HIGH>;
> +                       };
> +               };
> +
> +               i2c1c: i2c1@2 {
> +                       reg = <2>;
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       reg_soc_gpu_vpu: fan53555@60 {

regulator@60

> +&usdhc1 {
> +       bus-width = <8>;
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pinctrl_usdhc1>;
> +       non-removable;
> +       vqmmc-1-8-v;

This property does not exist. Please remove it.

^ permalink raw reply

* Re: [PATCH v3 2/7] drivers: Introduce device lookup variants by of_node
From: Lee Jones @ 2019-07-25 13:54 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: Andrew Lunn, Thor Thayer, rafael, Maxime Ripard, linux-fpga,
	dri-devel, linux-kernel, David S. Miller, Srinivas Kandagatla,
	linux-i2c, Frank Rowand, Florian Fainelli, linux-rockchip,
	Wolfram Sang, David Airlie, Jiri Slaby, devicetree, Alan Tull,
	Liam Girdwood, Rob Herring, Moritz Fischer, linux-arm-kernel,
	Mathieu Poirier, gregkh, Takashi Iwai, linux-spi
In-Reply-To: <20190723221838.12024-3-suzuki.poulose@arm.com>

On Tue, 23 Jul 2019, Suzuki K Poulose wrote:

> Introduce wrappers for {bus/driver/class}_find_device() to
> locate devices by its of_node.
> 
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: devicetree@vger.kernel.org
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: linux-i2c@vger.kernel.org
> Cc: linux-rockchip@lists.infradead.org
> Cc: linux-spi@vger.kernel.org
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Cc: Takashi Iwai <tiwai@suse.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: Alan Tull <atull@kernel.org>
> Cc: Moritz Fischer <mdf@kernel.org>
> Cc: linux-fpga@vger.kernel.org
> Cc: Peter Rosin <peda@axentia.se>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Heiner Kallweit <hkallweit1@gmail.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Thor Thayer <thor.thayer@linux.intel.com>
> Cc: Jiri Slaby <jslaby@suse.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Peter Rosin <peda@axentia.se>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  - Dropped the reviewed-by tags from Thor, Mark, Andrew and Peter as the
>    patches are mereged, though there are no functional changes.
> ---
>  drivers/amba/tegra-ahb.c              | 11 +-------
>  drivers/fpga/fpga-bridge.c            |  8 +-----
>  drivers/fpga/fpga-mgr.c               |  8 +-----
>  drivers/gpu/drm/drm_mipi_dsi.c        |  7 +----
>  drivers/i2c/i2c-core-of.c             |  7 +----
>  drivers/mfd/altera-sysmgr.c           | 14 ++--------

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

What's the merge plan for this patch?

Is anyone prepared to create an immutable branch for us to pull from?
I'm happy to do it if no one else steps up.

>  drivers/mux/core.c                    |  7 +----
>  drivers/net/phy/mdio_bus.c            |  9 +------
>  drivers/nvmem/core.c                  |  7 +----
>  drivers/of/of_mdio.c                  |  8 +-----
>  drivers/of/platform.c                 |  7 +----
>  drivers/regulator/of_regulator.c      |  7 +----
>  drivers/spi/spi.c                     | 20 +++------------
>  include/linux/device.h                | 37 +++++++++++++++++++++++++++
>  sound/soc/rockchip/rk3399_gru_sound.c |  9 ++-----
>  15 files changed, 56 insertions(+), 110 deletions(-)

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* [PATCH 3/3] arm64: dts: qcom: qcs404: remove unit name for thermal trip points
From: Vinod Koul @ 2019-07-25 13:51 UTC (permalink / raw)
  To: Andy Gross
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, Rob Herring,
	Mark Rutland, devicetree, linux-kernel
In-Reply-To: <20190725135150.9972-1-vkoul@kernel.org>

The thermal trip points have unit name but no reg property, so we can
remove them

arch/arm64/boot/dts/qcom/qcs404.dtsi:1080.31-1084.7: Warning (unit_address_vs_reg): /thermal-zones/aoss-thermal/trips/trip-point@0: node has a unit name, but no reg property
arch/arm64/boot/dts/qcom/qcs404.dtsi:1095.33-1099.7: Warning (unit_address_vs_reg): /thermal-zones/q6-hvx-thermal/trips/trip-point@0: node has a unit name, but no reg property
arch/arm64/boot/dts/qcom/qcs404.dtsi:1110.32-1114.7: Warning (unit_address_vs_reg): /thermal-zones/lpass-thermal/trips/trip-point@0: node has a unit name, but no reg property
arch/arm64/boot/dts/qcom/qcs404.dtsi:1125.31-1129.7: Warning (unit_address_vs_reg): /thermal-zones/wlan-thermal/trips/trip-point@0: node has a unit name, but no reg property
arch/arm64/boot/dts/qcom/qcs404.dtsi:1140.34-1144.7: Warning (unit_address_vs_reg): /thermal-zones/cluster-thermal/trips/trip-point@0: node has a unit name, but no reg property
arch/arm64/boot/dts/qcom/qcs404.dtsi:1145.34-1149.7: Warning (unit_address_vs_reg): /thermal-zones/cluster-thermal/trips/trip-point@1: node has a unit name, but no reg property
arch/arm64/boot/dts/qcom/qcs404.dtsi:1174.31-1178.7: Warning (unit_address_vs_reg): /thermal-zones/cpu0-thermal/trips/trip-point@0: node has a unit name, but no reg property
arch/arm64/boot/dts/qcom/qcs404.dtsi:1179.31-1183.7: Warning (unit_address_vs_reg): /thermal-zones/cpu0-thermal/trips/trip-point@1: node has a unit name, but no reg property
arch/arm64/boot/dts/qcom/qcs404.dtsi:1208.31-1212.7: Warning (unit_address_vs_reg): /thermal-zones/cpu1-thermal/trips/trip-point@0: node has a unit name, but no reg property
arch/arm64/boot/dts/qcom/qcs404.dtsi:1213.31-1217.7: Warning (unit_address_vs_reg): /thermal-zones/cpu1-thermal/trips/trip-point@1: node has a unit name, but no reg property
arch/arm64/boot/dts/qcom/qcs404.dtsi:1242.31-1246.7: Warning (unit_address_vs_reg): /thermal-zones/cpu2-thermal/trips/trip-point@0: node has a unit name, but no reg property
arch/arm64/boot/dts/qcom/qcs404.dtsi:1247.31-1251.7: Warning (unit_address_vs_reg): /thermal-zones/cpu2-thermal/trips/trip-point@1: node has a unit name, but no reg property
arch/arm64/boot/dts/qcom/qcs404.dtsi:1276.31-1280.7: Warning (unit_address_vs_reg): /thermal-zones/cpu3-thermal/trips/trip-point@0: node has a unit name, but no reg property
arch/arm64/boot/dts/qcom/qcs404.dtsi:1281.31-1285.7: Warning (unit_address_vs_reg): /thermal-zones/cpu3-thermal/trips/trip-point@1: node has a unit name, but no reg property
arch/arm64/boot/dts/qcom/qcs404.dtsi:1310.30-1314.7: Warning (unit_address_vs_reg): /thermal-zones/gpu-thermal/trips/trip-point@0: node has a unit name, but no reg property

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 arch/arm64/boot/dts/qcom/qcs404.dtsi | 30 ++++++++++++++--------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index 3d0789775009..6d91dae5aee0 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -1077,7 +1077,7 @@
 			thermal-sensors = <&tsens 0>;
 
 			trips {
-				aoss_alert0: trip-point@0 {
+				aoss_alert0: trip-point0 {
 					temperature = <105000>;
 					hysteresis = <2000>;
 					type = "hot";
@@ -1092,7 +1092,7 @@
 			thermal-sensors = <&tsens 1>;
 
 			trips {
-				q6_hvx_alert0: trip-point@0 {
+				q6_hvx_alert0: trip-point0 {
 					temperature = <105000>;
 					hysteresis = <2000>;
 					type = "hot";
@@ -1107,7 +1107,7 @@
 			thermal-sensors = <&tsens 2>;
 
 			trips {
-				lpass_alert0: trip-point@0 {
+				lpass_alert0: trip-point0 {
 					temperature = <105000>;
 					hysteresis = <2000>;
 					type = "hot";
@@ -1122,7 +1122,7 @@
 			thermal-sensors = <&tsens 3>;
 
 			trips {
-				wlan_alert0: trip-point@0 {
+				wlan_alert0: trip-point0 {
 					temperature = <105000>;
 					hysteresis = <2000>;
 					type = "hot";
@@ -1137,12 +1137,12 @@
 			thermal-sensors = <&tsens 4>;
 
 			trips {
-				cluster_alert0: trip-point@0 {
+				cluster_alert0: trip-point0 {
 					temperature = <95000>;
 					hysteresis = <2000>;
 					type = "hot";
 				};
-				cluster_alert1: trip-point@1 {
+				cluster_alert1: trip-point1 {
 					temperature = <105000>;
 					hysteresis = <2000>;
 					type = "passive";
@@ -1171,12 +1171,12 @@
 			thermal-sensors = <&tsens 5>;
 
 			trips {
-				cpu0_alert0: trip-point@0 {
+				cpu0_alert0: trip-point0 {
 					temperature = <95000>;
 					hysteresis = <2000>;
 					type = "hot";
 				};
-				cpu0_alert1: trip-point@1 {
+				cpu0_alert1: trip-point1 {
 					temperature = <105000>;
 					hysteresis = <2000>;
 					type = "passive";
@@ -1205,12 +1205,12 @@
 			thermal-sensors = <&tsens 6>;
 
 			trips {
-				cpu1_alert0: trip-point@0 {
+				cpu1_alert0: trip-point0 {
 					temperature = <95000>;
 					hysteresis = <2000>;
 					type = "hot";
 				};
-				cpu1_alert1: trip-point@1 {
+				cpu1_alert1: trip-point1 {
 					temperature = <105000>;
 					hysteresis = <2000>;
 					type = "passive";
@@ -1239,12 +1239,12 @@
 			thermal-sensors = <&tsens 7>;
 
 			trips {
-				cpu2_alert0: trip-point@0 {
+				cpu2_alert0: trip-point0 {
 					temperature = <95000>;
 					hysteresis = <2000>;
 					type = "hot";
 				};
-				cpu2_alert1: trip-point@1 {
+				cpu2_alert1: trip-point1 {
 					temperature = <105000>;
 					hysteresis = <2000>;
 					type = "passive";
@@ -1273,12 +1273,12 @@
 			thermal-sensors = <&tsens 8>;
 
 			trips {
-				cpu3_alert0: trip-point@0 {
+				cpu3_alert0: trip-point0 {
 					temperature = <95000>;
 					hysteresis = <2000>;
 					type = "hot";
 				};
-				cpu3_alert1: trip-point@1 {
+				cpu3_alert1: trip-point1 {
 					temperature = <105000>;
 					hysteresis = <2000>;
 					type = "passive";
@@ -1307,7 +1307,7 @@
 			thermal-sensors = <&tsens 9>;
 
 			trips {
-				gpu_alert0: trip-point@0 {
+				gpu_alert0: trip-point0 {
 					temperature = <95000>;
 					hysteresis = <2000>;
 					type = "hot";
-- 
2.20.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox