| Message ID | 20240123141311.220505-1-krzysztof.kozlowski@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | reset: gpio: ASoC: shared GPIO resets | expand |
On 24/01/24 03:13, Krzysztof Kozlowski wrote: > Hi, > > Patch #2 (cpufreq: do not open-code of_phandle_args_equal()) and patch #4 > (reset: Instantiate reset GPIO controller for shared reset-gpios) depend on OF > change (patch #1). > > Changes in v4 > ============= > 1. New patches: > of: add of_phandle_args_equal() helper > cpufreq: do not open-code of_phandle_args_equal() > > 2. reset-gpio.c: > - Drop unneeded comment (Bartosz), add Rb tag. > - Do not assign of_node. > > 3. reset/core.c: > - Implement most of Bartosz feedback (I responded to one which I did not > implement) and comments from Philipp. > - Expect either rcdev->of_args or rcdev->of_node. > - Drop __reset_gpios_args_match() and use common helper (Philipp). > - Move declarations of automatic-cleanup variables in > __reset_add_reset_gpio_lookup() to place of use (Bartosz). > - Separate gpio_device_get_label() and kstrdup() (Philipp). > - Correct doc for __reset_add_reset_gpio_device(), rewrite few comments. > - Drop unneeded "r" variable in __reset_find_rcdev() (Philipp). > - Drop of_phandle_args initialization in __of_reset_control_get (Philipp). > - Check if CONFIG_RESET_GPIO is enabled before trying to look up reset-gpios. > > 4. Drop Chris' patch: "i2c: muxes: pca954x: Allow sharing reset GPIO", because > discussion is on going. I actually think it would have been OK as-is with your latest change to return NULL when CONFIG_RESET_GPIO is not enabled. But I'm happy to submit it independently after this series lands. It'll give me a chance to do a bit more testing.
On 23/01/2024 20:28, Chris Packham wrote: > > On 24/01/24 03:13, Krzysztof Kozlowski wrote: >> Hi, >> >> Patch #2 (cpufreq: do not open-code of_phandle_args_equal()) and patch #4 >> (reset: Instantiate reset GPIO controller for shared reset-gpios) depend on OF >> change (patch #1). >> >> Changes in v4 >> ============= >> 1. New patches: >> of: add of_phandle_args_equal() helper >> cpufreq: do not open-code of_phandle_args_equal() >> >> 2. reset-gpio.c: >> - Drop unneeded comment (Bartosz), add Rb tag. >> - Do not assign of_node. >> >> 3. reset/core.c: >> - Implement most of Bartosz feedback (I responded to one which I did not >> implement) and comments from Philipp. >> - Expect either rcdev->of_args or rcdev->of_node. >> - Drop __reset_gpios_args_match() and use common helper (Philipp). >> - Move declarations of automatic-cleanup variables in >> __reset_add_reset_gpio_lookup() to place of use (Bartosz). >> - Separate gpio_device_get_label() and kstrdup() (Philipp). >> - Correct doc for __reset_add_reset_gpio_device(), rewrite few comments. >> - Drop unneeded "r" variable in __reset_find_rcdev() (Philipp). >> - Drop of_phandle_args initialization in __of_reset_control_get (Philipp). >> - Check if CONFIG_RESET_GPIO is enabled before trying to look up reset-gpios. >> >> 4. Drop Chris' patch: "i2c: muxes: pca954x: Allow sharing reset GPIO", because >> discussion is on going. > > I actually think it would have been OK as-is with your latest change to > return NULL when CONFIG_RESET_GPIO is not enabled. But I'm happy to > submit it independently after this series lands. It'll give me a chance > to do a bit more testing. Yeah, I wasn't sure if the concerns are really resolved. I think you can send your patch separately because there is no clear dependency, so also no clear benefits of me taking your patch into this patchset. Best regards, Krzysztof
Hi, Patch #2 (cpufreq: do not open-code of_phandle_args_equal()) and patch #4 (reset: Instantiate reset GPIO controller for shared reset-gpios) depend on OF change (patch #1). Changes in v4 ============= 1. New patches: of: add of_phandle_args_equal() helper cpufreq: do not open-code of_phandle_args_equal() 2. reset-gpio.c: - Drop unneeded comment (Bartosz), add Rb tag. - Do not assign of_node. 3. reset/core.c: - Implement most of Bartosz feedback (I responded to one which I did not implement) and comments from Philipp. - Expect either rcdev->of_args or rcdev->of_node. - Drop __reset_gpios_args_match() and use common helper (Philipp). - Move declarations of automatic-cleanup variables in __reset_add_reset_gpio_lookup() to place of use (Bartosz). - Separate gpio_device_get_label() and kstrdup() (Philipp). - Correct doc for __reset_add_reset_gpio_device(), rewrite few comments. - Drop unneeded "r" variable in __reset_find_rcdev() (Philipp). - Drop of_phandle_args initialization in __of_reset_control_get (Philipp). - Check if CONFIG_RESET_GPIO is enabled before trying to look up reset-gpios. 4. Drop Chris' patch: "i2c: muxes: pca954x: Allow sharing reset GPIO", because discussion is on going. Changes in v3 ============= 1. reset-gpio.c: - Add reset_gpio_of_xlate (Philipp). - reset_gpio_of_args_put->reset_gpio_of_node_put (Philipp). - Expect via platdata of_phandle_args. - Do not call device_set_node() to attach itself to reset consumer (the final device). This was questionable idea in the first place. Bartosz suggested to use GPIO_LOOKUP to solve this. 2. reset/core.c, implement Philipp's feedback. That was a lot: - Commit msg fixes. - Add new platform_device earlier, when reset core found "reset-gpios" but not "resets". - Do not overwrite of_phandle_args. - Expect matching .of_reset_n_cells. - Pass of_phandle_args as platdata to reset-gpio. - Rename reset_gpio_device->reset_gpio_lookup and others. Fix few comments and code cleanup pointed on review. - From Bartosz: Use GPIO_LOOKUP and a lot of cleanup.h in __reset_add_reset_gpio_lookup(). 3. Include here Chris' patch: "i2c: muxes: pca954x: Allow sharing reset GPIO". Changes in v2 ============= 1. wsa884x.c: add missing return in wsa884x_get_reset(), correct comment. 2. qcom,wsa8840.yaml: fix oneOf syntax. 3. reset-gpio.c: - Fix smatch warning on platdata evaluation. - Parse GPIO args and store them in rc.of_args. 4. reset/core.c: - Revise approach based on Bartosz comments: parse the reset-gpios phandle with arguments, do not use deprecated API and do not rely on gpio_desc pointer. - Create a list of instantiated platform devices to avoid any duplicates. - After creating reset-gpio platform device, try to get new reset controller or return EPROBE_DEFER. - Drop the "cookie" member and add new "of_args" to "struct reset_controller_dev". Description =========== We have at least few cases where hardware engineers decided to use one powerdown/shutdown/reset GPIO line for multiple devices: 1. WSA884x (this and previous patch): https://lore.kernel.org/all/b7aeda24-d638-45b7-8e30-80d287f498f8@sirena.org.uk/ 2. https://lore.kernel.org/all/20231027033104.1348921-1-chris.packham@alliedtelesis.co.nz/ 3. https://lore.kernel.org/lkml/20191030120440.3699-1-peter.ujfalusi@ti.com/ 4. https://lore.kernel.org/all/20211018234923.1769028-1-sean.anderson@seco.com/ 5. https://social.treehouse.systems/@marcan/111268780311634160 I try to solve my case, hopefuly Chris' (2), partially Sean's (4) and maybe Hectors (5), using Rob's suggestion: https://lore.kernel.org/all/YXi5CUCEi7YmNxXM@robh.at.kernel.org/ Best regards, Krzysztof Cc: Chris Packham <chris.packham@alliedtelesis.co.nz> Cc: Bartosz Golaszewski <brgl@bgdev.pl> Cc: Sean Anderson <sean.anderson@seco.com> Krzysztof Kozlowski (6): of: Add of_phandle_args_equal() helper cpufreq: do not open-code of_phandle_args_equal() reset: gpio: Add GPIO-based reset controller reset: Instantiate reset GPIO controller for shared reset-gpios ASoC: dt-bindings: qcom,wsa8840: Add reset-gpios for shared line ASoC: codecs: wsa884x: Allow sharing reset GPIO .../bindings/sound/qcom,wsa8840.yaml | 11 +- MAINTAINERS | 5 + drivers/reset/Kconfig | 9 + drivers/reset/Makefile | 1 + drivers/reset/core.c | 213 ++++++++++++++++-- drivers/reset/reset-gpio.c | 119 ++++++++++ include/linux/cpufreq.h | 3 +- include/linux/of.h | 16 ++ include/linux/reset-controller.h | 4 + sound/soc/codecs/wsa884x.c | 53 ++++- 10 files changed, 408 insertions(+), 26 deletions(-) create mode 100644 drivers/reset/reset-gpio.c