2024-02-23 04:11:58

by Yang Xiwen via B4 Relay

[permalink] [raw]
Subject: [PATCH 0/2] simple-pm-bus: deassert resets if possible

simple power-managed buses can also have resets. Get and deassert them
if possible.

Signed-off-by: Yang Xiwen <[email protected]>
---
Yang Xiwen (2):
dt-bindings: simple-pm-bus: Add optional resets
drivers: bus: simple-pm-bus: Get and deassert resets exclusively

Documentation/devicetree/bindings/bus/simple-pm-bus.yaml | 7 +++++--
drivers/bus/simple-pm-bus.c | 14 +++++++++++++-
2 files changed, 18 insertions(+), 3 deletions(-)
---
base-commit: 8d3dea210042f54b952b481838c1e7dfc4ec751d
change-id: 20240223-b4-bus-d5c6f75a251a

Best regards,
--
Yang Xiwen <[email protected]>



2024-02-23 04:12:04

by Yang Xiwen via B4 Relay

[permalink] [raw]
Subject: [PATCH 2/2] drivers: bus: simple-pm-bus: Get and deassert resets exclusively

From: Yang Xiwen <[email protected]>

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 <[email protected]>
---
drivers/bus/simple-pm-bus.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

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;
}


--
2.43.0


2024-02-23 08:08:45

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 2/2] drivers: bus: simple-pm-bus: Get and deassert resets exclusively

Hi Yang,

On Fri, Feb 23, 2024 at 4:49 AM Yang Xiwen via B4 Relay
<[email protected]> wrote:
> From: Yang Xiwen <[email protected]>
>
> 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 <[email protected]>

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 <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2024-02-23 22:41:22

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] drivers: bus: simple-pm-bus: Get and deassert resets exclusively

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/[email protected]/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/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-02-23 22:51:46

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] drivers: bus: simple-pm-bus: Get and deassert resets exclusively

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/[email protected]/config)
compiler: or1k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240224/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-02-23 23:33:13

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] drivers: bus: simple-pm-bus: Get and deassert resets exclusively

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/[email protected]/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/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

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

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-02-24 12:12:59

by Yang Xiwen

[permalink] [raw]
Subject: Re: [PATCH 2/2] drivers: bus: simple-pm-bus: Get and deassert resets exclusively

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/[email protected]/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/[email protected]/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 <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> 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
>

--
Regards,
Yang Xiwen