diff mbox series

[2/2] drivers: bus: simple-pm-bus: Get and deassert resets exclusively

Message ID 20240223-b4-bus-v1-2-2803c3ac4673@outlook.com
State New
Headers
Series simple-pm-bus: deassert resets if possible |

Commit Message

Yang Xiwen via B4 Relay Feb. 23, 2024, 3:49 a.m. UTC 
  From: Yang Xiwen <forbidden405@outlook.com>

Simple Power-Managed bus controller may need functional reset(s)
to be deasserted before child devices connected to the bus can be
accessed.  Get the reset(s) as an array and assert/deassert the
reset(s) when the bus is being power managed.

One example is that HiSilicon USB2 INNO PHY test bus needs to deassert
the reset to the bus before accessing its registers.

Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
---
 drivers/bus/simple-pm-bus.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Geert Uytterhoeven Feb. 23, 2024, 8:08 a.m. UTC | #1 
Hi Yang,

On Fri, Feb 23, 2024 at 4:49 AM Yang Xiwen via B4 Relay
<devnull+forbidden405.outlook.com@kernel.org> wrote:
> From: Yang Xiwen <forbidden405@outlook.com>
>
> Simple Power-Managed bus controller may need functional reset(s)
> to be deasserted before child devices connected to the bus can be
> accessed.  Get the reset(s) as an array and assert/deassert the
> reset(s) when the bus is being power managed.
>
> One example is that HiSilicon USB2 INNO PHY test bus needs to deassert
> the reset to the bus before accessing its registers.
>
> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>

Thanks for your patch!

> --- a/drivers/bus/simple-pm-bus.c
> +++ b/drivers/bus/simple-pm-bus.c
> @@ -62,6 +64,10 @@ static int simple_pm_bus_probe(struct platform_device *pdev)
>         if (bus->num_clks < 0)
>                 return dev_err_probe(&pdev->dev, bus->num_clks, "failed to get clocks\n");
>
> +       bus->rsts = devm_reset_control_array_get(dev, false, true);

Please use the devm_reset_control_array_get_optional_exclusive()
wrapper.

> +       if (IS_ERR(bus->rsts))
> +               return dev_err_probe(&pdev->dev, PTR_ERR(bus->rsts), "failed to get resets\n");
> +
>         dev_set_drvdata(&pdev->dev, bus);
>
>         dev_dbg(&pdev->dev, "%s\n", __func__);

The rest LGTM, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert
kernel test robot Feb. 23, 2024, 10:40 p.m. UTC | #2 
Hi Yang,

kernel test robot noticed the following build errors:

[auto build test ERROR on 8d3dea210042f54b952b481838c1e7dfc4ec751d]

url:    https://github.com/intel-lab-lkp/linux/commits/Yang-Xiwen-via-B4-Relay/dt-bindings-simple-pm-bus-Add-optional-resets/20240223-115400
base:   8d3dea210042f54b952b481838c1e7dfc4ec751d
patch link:    https://lore.kernel.org/r/20240223-b4-bus-v1-2-2803c3ac4673%40outlook.com
patch subject: [PATCH 2/2] drivers: bus: simple-pm-bus: Get and deassert resets exclusively
config: arm-allnoconfig (https://download.01.org/0day-ci/archive/20240224/202402240646.IirqjDw8-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project edd4aee4dd9b5b98b2576a6f783e4086173d902a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240224/202402240646.IirqjDw8-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402240646.IirqjDw8-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/bus/simple-pm-bus.c:67:43: error: passing 'const struct device *' to parameter of type 'struct device *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
      67 |         bus->rsts = devm_reset_control_array_get(dev, false, true);
         |                                                  ^~~
   include/linux/reset.h:192:45: note: passing argument to parameter 'dev' here
     192 | devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
         |                                             ^
   1 error generated.


vim +67 drivers/bus/simple-pm-bus.c

    26	
    27	static int simple_pm_bus_probe(struct platform_device *pdev)
    28	{
    29		const struct device *dev = &pdev->dev;
    30		const struct of_dev_auxdata *lookup = dev_get_platdata(dev);
    31		struct device_node *np = dev->of_node;
    32		const struct of_device_id *match;
    33		struct simple_pm_bus *bus;
    34	
    35		/*
    36		 * Allow user to use driver_override to bind this driver to a
    37		 * transparent bus device which has a different compatible string
    38		 * that's not listed in simple_pm_bus_of_match. We don't want to do any
    39		 * of the simple-pm-bus tasks for these devices, so return early.
    40		 */
    41		if (pdev->driver_override)
    42			return 0;
    43	
    44		match = of_match_device(dev->driver->of_match_table, dev);
    45		/*
    46		 * These are transparent bus devices (not simple-pm-bus matches) that
    47		 * have their child nodes populated automatically.  So, don't need to
    48		 * do anything more. We only match with the device if this driver is
    49		 * the most specific match because we don't want to incorrectly bind to
    50		 * a device that has a more specific driver.
    51		 */
    52		if (match && match->data) {
    53			if (of_property_match_string(np, "compatible", match->compatible) == 0)
    54				return 0;
    55			else
    56				return -ENODEV;
    57		}
    58	
    59		bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
    60		if (!bus)
    61			return -ENOMEM;
    62	
    63		bus->num_clks = devm_clk_bulk_get_all(&pdev->dev, &bus->clks);
    64		if (bus->num_clks < 0)
    65			return dev_err_probe(&pdev->dev, bus->num_clks, "failed to get clocks\n");
    66	
  > 67		bus->rsts = devm_reset_control_array_get(dev, false, true);
    68		if (IS_ERR(bus->rsts))
    69			return dev_err_probe(&pdev->dev, PTR_ERR(bus->rsts), "failed to get resets\n");
    70	
    71		dev_set_drvdata(&pdev->dev, bus);
    72	
    73		dev_dbg(&pdev->dev, "%s\n", __func__);
    74	
    75		pm_runtime_enable(&pdev->dev);
    76	
    77		if (np)
    78			of_platform_populate(np, NULL, lookup, &pdev->dev);
    79	
    80		return 0;
    81	}
    82
kernel test robot Feb. 23, 2024, 10:50 p.m. UTC | #3 
Hi Yang,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 8d3dea210042f54b952b481838c1e7dfc4ec751d]

url:    https://github.com/intel-lab-lkp/linux/commits/Yang-Xiwen-via-B4-Relay/dt-bindings-simple-pm-bus-Add-optional-resets/20240223-115400
base:   8d3dea210042f54b952b481838c1e7dfc4ec751d
patch link:    https://lore.kernel.org/r/20240223-b4-bus-v1-2-2803c3ac4673%40outlook.com
patch subject: [PATCH 2/2] drivers: bus: simple-pm-bus: Get and deassert resets exclusively
config: openrisc-allnoconfig (https://download.01.org/0day-ci/archive/20240224/202402240650.1O7vtd48-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240224/202402240650.1O7vtd48-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402240650.1O7vtd48-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/bus/simple-pm-bus.c: In function 'simple_pm_bus_probe':
>> drivers/bus/simple-pm-bus.c:67:50: warning: passing argument 1 of 'devm_reset_control_array_get' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      67 |         bus->rsts = devm_reset_control_array_get(dev, false, true);
         |                                                  ^~~
   In file included from drivers/bus/simple-pm-bus.c:19:
   include/linux/reset.h:192:45: note: expected 'struct device *' but argument is of type 'const struct device *'
     192 | devm_reset_control_array_get(struct device *dev, bool shared, bool optional)
         |                              ~~~~~~~~~~~~~~~^~~


vim +67 drivers/bus/simple-pm-bus.c

    26	
    27	static int simple_pm_bus_probe(struct platform_device *pdev)
    28	{
    29		const struct device *dev = &pdev->dev;
    30		const struct of_dev_auxdata *lookup = dev_get_platdata(dev);
    31		struct device_node *np = dev->of_node;
    32		const struct of_device_id *match;
    33		struct simple_pm_bus *bus;
    34	
    35		/*
    36		 * Allow user to use driver_override to bind this driver to a
    37		 * transparent bus device which has a different compatible string
    38		 * that's not listed in simple_pm_bus_of_match. We don't want to do any
    39		 * of the simple-pm-bus tasks for these devices, so return early.
    40		 */
    41		if (pdev->driver_override)
    42			return 0;
    43	
    44		match = of_match_device(dev->driver->of_match_table, dev);
    45		/*
    46		 * These are transparent bus devices (not simple-pm-bus matches) that
    47		 * have their child nodes populated automatically.  So, don't need to
    48		 * do anything more. We only match with the device if this driver is
    49		 * the most specific match because we don't want to incorrectly bind to
    50		 * a device that has a more specific driver.
    51		 */
    52		if (match && match->data) {
    53			if (of_property_match_string(np, "compatible", match->compatible) == 0)
    54				return 0;
    55			else
    56				return -ENODEV;
    57		}
    58	
    59		bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
    60		if (!bus)
    61			return -ENOMEM;
    62	
    63		bus->num_clks = devm_clk_bulk_get_all(&pdev->dev, &bus->clks);
    64		if (bus->num_clks < 0)
    65			return dev_err_probe(&pdev->dev, bus->num_clks, "failed to get clocks\n");
    66	
  > 67		bus->rsts = devm_reset_control_array_get(dev, false, true);
    68		if (IS_ERR(bus->rsts))
    69			return dev_err_probe(&pdev->dev, PTR_ERR(bus->rsts), "failed to get resets\n");
    70	
    71		dev_set_drvdata(&pdev->dev, bus);
    72	
    73		dev_dbg(&pdev->dev, "%s\n", __func__);
    74	
    75		pm_runtime_enable(&pdev->dev);
    76	
    77		if (np)
    78			of_platform_populate(np, NULL, lookup, &pdev->dev);
    79	
    80		return 0;
    81	}
    82
kernel test robot Feb. 23, 2024, 11:32 p.m. UTC | #4 
Hi Yang,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 8d3dea210042f54b952b481838c1e7dfc4ec751d]

url:    https://github.com/intel-lab-lkp/linux/commits/Yang-Xiwen-via-B4-Relay/dt-bindings-simple-pm-bus-Add-optional-resets/20240223-115400
base:   8d3dea210042f54b952b481838c1e7dfc4ec751d
patch link:    https://lore.kernel.org/r/20240223-b4-bus-v1-2-2803c3ac4673%40outlook.com
patch subject: [PATCH 2/2] drivers: bus: simple-pm-bus: Get and deassert resets exclusively
config: i386-randconfig-061-20240223 (https://download.01.org/0day-ci/archive/20240224/202402240740.rPfLNrGO-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240224/202402240740.rPfLNrGO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402240740.rPfLNrGO-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/bus/simple-pm-bus.c:67:50: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected struct device *dev @@     got struct device const *dev @@
   drivers/bus/simple-pm-bus.c:67:50: sparse:     expected struct device *dev
   drivers/bus/simple-pm-bus.c:67:50: sparse:     got struct device const *dev

vim +67 drivers/bus/simple-pm-bus.c

    26	
    27	static int simple_pm_bus_probe(struct platform_device *pdev)
    28	{
    29		const struct device *dev = &pdev->dev;
    30		const struct of_dev_auxdata *lookup = dev_get_platdata(dev);
    31		struct device_node *np = dev->of_node;
    32		const struct of_device_id *match;
    33		struct simple_pm_bus *bus;
    34	
    35		/*
    36		 * Allow user to use driver_override to bind this driver to a
    37		 * transparent bus device which has a different compatible string
    38		 * that's not listed in simple_pm_bus_of_match. We don't want to do any
    39		 * of the simple-pm-bus tasks for these devices, so return early.
    40		 */
    41		if (pdev->driver_override)
    42			return 0;
    43	
    44		match = of_match_device(dev->driver->of_match_table, dev);
    45		/*
    46		 * These are transparent bus devices (not simple-pm-bus matches) that
    47		 * have their child nodes populated automatically.  So, don't need to
    48		 * do anything more. We only match with the device if this driver is
    49		 * the most specific match because we don't want to incorrectly bind to
    50		 * a device that has a more specific driver.
    51		 */
    52		if (match && match->data) {
    53			if (of_property_match_string(np, "compatible", match->compatible) == 0)
    54				return 0;
    55			else
    56				return -ENODEV;
    57		}
    58	
    59		bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
    60		if (!bus)
    61			return -ENOMEM;
    62	
    63		bus->num_clks = devm_clk_bulk_get_all(&pdev->dev, &bus->clks);
    64		if (bus->num_clks < 0)
    65			return dev_err_probe(&pdev->dev, bus->num_clks, "failed to get clocks\n");
    66	
  > 67		bus->rsts = devm_reset_control_array_get(dev, false, true);
    68		if (IS_ERR(bus->rsts))
    69			return dev_err_probe(&pdev->dev, PTR_ERR(bus->rsts), "failed to get resets\n");
    70	
    71		dev_set_drvdata(&pdev->dev, bus);
    72	
    73		dev_dbg(&pdev->dev, "%s\n", __func__);
    74	
    75		pm_runtime_enable(&pdev->dev);
    76	
    77		if (np)
    78			of_platform_populate(np, NULL, lookup, &pdev->dev);
    79	
    80		return 0;
    81	}
    82
Yang Xiwen Feb. 24, 2024, 12:12 p.m. UTC | #5 
On 2/24/2024 7:32 AM, kernel test robot wrote:
> Hi Yang,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on 8d3dea210042f54b952b481838c1e7dfc4ec751d]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Yang-Xiwen-via-B4-Relay/dt-bindings-simple-pm-bus-Add-optional-resets/20240223-115400
> base:   8d3dea210042f54b952b481838c1e7dfc4ec751d
> patch link:    https://lore.kernel.org/r/20240223-b4-bus-v1-2-2803c3ac4673%40outlook.com
> patch subject: [PATCH 2/2] drivers: bus: simple-pm-bus: Get and deassert resets exclusively
> config: i386-randconfig-061-20240223 (https://download.01.org/0day-ci/archive/20240224/202402240740.rPfLNrGO-lkp@intel.com/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240224/202402240740.rPfLNrGO-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202402240740.rPfLNrGO-lkp@intel.com/
>
> sparse warnings: (new ones prefixed by >>)
>>> drivers/bus/simple-pm-bus.c:67:50: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected struct device *dev @@     got struct device const *dev @@
>     drivers/bus/simple-pm-bus.c:67:50: sparse:     expected struct device *dev
>     drivers/bus/simple-pm-bus.c:67:50: sparse:     got struct device const *dev
>
> vim +67 drivers/bus/simple-pm-bus.c
>
>      26	
>      27	static int simple_pm_bus_probe(struct platform_device *pdev)
>      28	{
>      29		const struct device *dev = &pdev->dev;
>      30		const struct of_dev_auxdata *lookup = dev_get_platdata(dev);
>      31		struct device_node *np = dev->of_node;
>      32		const struct of_device_id *match;
>      33		struct simple_pm_bus *bus;
>      34	
>      35		/*
>      36		 * Allow user to use driver_override to bind this driver to a
>      37		 * transparent bus device which has a different compatible string
>      38		 * that's not listed in simple_pm_bus_of_match. We don't want to do any
>      39		 * of the simple-pm-bus tasks for these devices, so return early.
>      40		 */
>      41		if (pdev->driver_override)
>      42			return 0;
>      43	
>      44		match = of_match_device(dev->driver->of_match_table, dev);
>      45		/*
>      46		 * These are transparent bus devices (not simple-pm-bus matches) that
>      47		 * have their child nodes populated automatically.  So, don't need to
>      48		 * do anything more. We only match with the device if this driver is
>      49		 * the most specific match because we don't want to incorrectly bind to
>      50		 * a device that has a more specific driver.
>      51		 */
>      52		if (match && match->data) {
>      53			if (of_property_match_string(np, "compatible", match->compatible) == 0)
>      54				return 0;
>      55			else
>      56				return -ENODEV;
>      57		}
>      58	
>      59		bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
>      60		if (!bus)
>      61			return -ENOMEM;
>      62	
>      63		bus->num_clks = devm_clk_bulk_get_all(&pdev->dev, &bus->clks);
>      64		if (bus->num_clks < 0)
>      65			return dev_err_probe(&pdev->dev, bus->num_clks, "failed to get clocks\n");
>      66	
>    > 67		bus->rsts = devm_reset_control_array_get(dev, false, true);


Fixed in v2 before reporting.


>      68		if (IS_ERR(bus->rsts))
>      69			return dev_err_probe(&pdev->dev, PTR_ERR(bus->rsts), "failed to get resets\n");
>      70	
>      71		dev_set_drvdata(&pdev->dev, bus);
>      72	
>      73		dev_dbg(&pdev->dev, "%s\n", __func__);
>      74	
>      75		pm_runtime_enable(&pdev->dev);
>      76	
>      77		if (np)
>      78			of_platform_populate(np, NULL, lookup, &pdev->dev);
>      79	
>      80		return 0;
>      81	}
>      82	
>
diff mbox series

Patch

diff --git a/drivers/bus/simple-pm-bus.c b/drivers/bus/simple-pm-bus.c
index 50870c827889..d84dbd61c02b 100644
--- a/drivers/bus/simple-pm-bus.c
+++ b/drivers/bus/simple-pm-bus.c
@@ -16,9 +16,11 @@ 
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/reset.h>
 
 struct simple_pm_bus {
 	struct clk_bulk_data *clks;
+	struct reset_control *rsts;
 	int num_clks;
 };
 
@@ -62,6 +64,10 @@  static int simple_pm_bus_probe(struct platform_device *pdev)
 	if (bus->num_clks < 0)
 		return dev_err_probe(&pdev->dev, bus->num_clks, "failed to get clocks\n");
 
+	bus->rsts = devm_reset_control_array_get(dev, false, true);
+	if (IS_ERR(bus->rsts))
+		return dev_err_probe(&pdev->dev, PTR_ERR(bus->rsts), "failed to get resets\n");
+
 	dev_set_drvdata(&pdev->dev, bus);
 
 	dev_dbg(&pdev->dev, "%s\n", __func__);
@@ -92,7 +98,7 @@  static int simple_pm_bus_runtime_suspend(struct device *dev)
 
 	clk_bulk_disable_unprepare(bus->num_clks, bus->clks);
 
-	return 0;
+	return reset_control_assert(bus->rsts);
 }
 
 static int simple_pm_bus_runtime_resume(struct device *dev)
@@ -106,6 +112,12 @@  static int simple_pm_bus_runtime_resume(struct device *dev)
 		return ret;
 	}
 
+	ret = reset_control_deassert(bus->rsts);
+	if (ret) {
+		dev_err(dev, "failed to deassert resets: %d\n", ret);
+		return ret;
+	}
+
 	return 0;
 }