| Message ID | 20220608095623.22327-12-tmaimon77@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Introduce Nuvoton Arbel NPCM8XX BMC SoC | expand |
Hi Krzysztof Sorry but I didn't ignore your comment. For not breaking exciting boards I add the following patch in V2 https://lore.kernel.org/linux-arm-kernel/20220608095623.22327-11-tmaimon77@gmail.com/ On Wed, 8 Jun 2022 at 13:08, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 08/06/2022 11:56, Tomer Maimon wrote: > > Using syscon device tree property instead of > > device data to handle the NPCM general control > > registers. > > > > Again ignored the comment. > > > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com> > > --- > > drivers/reset/reset-npcm.c | 11 +++-------- > > 1 file changed, 3 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/reset/reset-npcm.c b/drivers/reset/reset-npcm.c > > index 2ea4d3136e15..312c3b594b8f 100644 > > --- a/drivers/reset/reset-npcm.c > > +++ b/drivers/reset/reset-npcm.c > > @@ -138,8 +138,7 @@ static int npcm_reset_xlate(struct reset_controller_dev *rcdev, > > } > > > > static const struct of_device_id npcm_rc_match[] = { > > - { .compatible = "nuvoton,npcm750-reset", > > - .data = (void *)"nuvoton,npcm750-gcr" }, > > + { .compatible = "nuvoton,npcm750-reset"}, > > { } > > }; > > > > @@ -155,14 +154,10 @@ static int npcm_usb_reset(struct platform_device *pdev, struct npcm_rc_data *rc) > > u32 ipsrst1_bits = 0; > > u32 ipsrst2_bits = NPCM_IPSRST2_USB_HOST; > > u32 ipsrst3_bits = 0; > > - const char *gcr_dt; > > > > - gcr_dt = (const char *) > > - of_match_device(dev->driver->of_match_table, dev)->data; > > - > > - gcr_regmap = syscon_regmap_lookup_by_compatible(gcr_dt); > > + gcr_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "nuvoton,sysgcr"); > > if (IS_ERR(gcr_regmap)) { > > - dev_err(&pdev->dev, "Failed to find %s\n", gcr_dt); > > + dev_err(&pdev->dev, "Failed to find gcr syscon"); > > return PTR_ERR(gcr_regmap); > > Comment still ignored. > > There is no point in this review if you keep ignoring what we ask to fix. > > If something is unclear, ask for clarification. Resending without > implementing the comment means that you ignore the review which is waste > of my time. > > I am sorry, but this is not acceptable. > > Best regards, > Krzysztof Best regards, Tomer
On 09/06/2022 23:37, Tomer Maimon wrote: > Hi Krzysztof > > Sorry but I didn't ignore your comment. > > For not breaking exciting boards I add the following patch in V2 > https://lore.kernel.org/linux-arm-kernel/20220608095623.22327-11-tmaimon77@gmail.com/ No, it does not solve it. 1. Patchset goes via separate trees (DTS are always separate), so it is not bisectable. One of the branches/trees will have broken DTS. 2. All out of tree DTSes are broken. This is expressed as ABI and - with some reasonable exceptions - you should not break it. https://elixir.bootlin.com/linux/v5.19-rc1/source/Documentation/devicetree/bindings/ABI.rst You have to keep backwards compatibility, so parse/handle both versions of DTS. Best regards, Krzysztof
Hi Krzysztof, I will make sure to add backward compatibility in the reset driver in the next version. Thanks, Tomer On Fri, 10 Jun 2022 at 12:53, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 09/06/2022 23:37, Tomer Maimon wrote: > > Hi Krzysztof > > > > Sorry but I didn't ignore your comment. > > > > For not breaking exciting boards I add the following patch in V2 > > https://lore.kernel.org/linux-arm-kernel/20220608095623.22327-11-tmaimon77@gmail.com/ > > No, it does not solve it. > 1. Patchset goes via separate trees (DTS are always separate), so it is > not bisectable. One of the branches/trees will have broken DTS. > > 2. All out of tree DTSes are broken. This is expressed as ABI and - with > some reasonable exceptions - you should not break it. > https://elixir.bootlin.com/linux/v5.19-rc1/source/Documentation/devicetree/bindings/ABI.rst > > You have to keep backwards compatibility, so parse/handle both versions > of DTS. > > > Best regards, > Krzysztof
diff --git a/drivers/reset/reset-npcm.c b/drivers/reset/reset-npcm.c index 2ea4d3136e15..312c3b594b8f 100644 --- a/drivers/reset/reset-npcm.c +++ b/drivers/reset/reset-npcm.c @@ -138,8 +138,7 @@ static int npcm_reset_xlate(struct reset_controller_dev *rcdev, } static const struct of_device_id npcm_rc_match[] = { - { .compatible = "nuvoton,npcm750-reset", - .data = (void *)"nuvoton,npcm750-gcr" }, + { .compatible = "nuvoton,npcm750-reset"}, { } }; @@ -155,14 +154,10 @@ static int npcm_usb_reset(struct platform_device *pdev, struct npcm_rc_data *rc) u32 ipsrst1_bits = 0; u32 ipsrst2_bits = NPCM_IPSRST2_USB_HOST; u32 ipsrst3_bits = 0; - const char *gcr_dt; - gcr_dt = (const char *) - of_match_device(dev->driver->of_match_table, dev)->data; - - gcr_regmap = syscon_regmap_lookup_by_compatible(gcr_dt); + gcr_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "nuvoton,sysgcr"); if (IS_ERR(gcr_regmap)) { - dev_err(&pdev->dev, "Failed to find %s\n", gcr_dt); + dev_err(&pdev->dev, "Failed to find gcr syscon"); return PTR_ERR(gcr_regmap); }
Using syscon device tree property instead of device data to handle the NPCM general control registers. Signed-off-by: Tomer Maimon <tmaimon77@gmail.com> --- drivers/reset/reset-npcm.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)