| Message ID | 20220928-mdm9615-dt-schema-fixes-v1-0-b6e63a7df1e8@linaro.org |
|---|---|
| Headers | show |
| Series | arm: qcom: mdm9615: first round of bindings and DT fixes | expand |
On 28/09/2022 11:14, Neil Armstrong wrote: > Replace the licence blob by a clean SPDX-License-Identifier > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > Rebase on linux-next - you use old Bjorn's email address. > diff --git a/arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dts b/arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dts > index 0827de5426c1..073c15354483 100644 > --- a/arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dts > +++ b/arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dts > @@ -1,46 +1,9 @@ > +// SPDX-License-Identifier: GPL-2.0+ OR X11 > /* > * Device Tree Source for mangOH Green Board with WP8548 Module > * > * Copyright (C) 2016 BayLibre, SAS. > * Author : Neil Armstrong <narmstrong@baylibre.com> > - * > - * This file is dual-licensed: you can use it either under the terms > - * of the GPL or the X11 license, at your option. Note that this dual > - * licensing only applies to this file, and not this project as a > - * whole. > - * > - * a) This file is free software; you can redistribute it and/or > - * modify it under the terms of the GNU General Public License as > - * published by the Free Software Foundation; either version 2 of the > - * License, or (at your option) any later version. > - * > - * This file is distributed in the hope that it will be useful, > - * but WITHOUT ANY WARRANTY; without even the implied warranty of > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > - * GNU General Public License for more details. > - * > - * Or, alternatively, > - * > - * b) Permission is hereby granted, free of charge, to any person > - * obtaining a copy of this software and associated documentation > - * files (the "Software"), to deal in the Software without > - * restriction, including without limitation the rights to use, > - * copy, modify, merge, publish, distribute, sublicense, and/or > - * sell copies of the Software, and to permit persons to whom the > - * Software is furnished to do so, subject to the following > - * conditions: > - * > - * The above copyright notice and this permission notice shall be > - * included in all copies or substantial portions of the Software. > - * > - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES > - * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND > - * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT > - * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, > - * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > - * OTHER DEALINGS IN THE SOFTWARE. The text is actually MIT, not X11. I think they differ by last X11 trademark statement. Best regards, Krzysztof
On 28/09/2022 11:14, Neil Armstrong wrote: > Fixes cpu@0: 'reg' is a required property from dtbs check. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > > diff --git a/arch/arm/boot/dts/qcom-mdm9615.dtsi b/arch/arm/boot/dts/qcom-mdm9615.dtsi > index b06bbe25fdd4..e547becc9f75 100644 > --- a/arch/arm/boot/dts/qcom-mdm9615.dtsi > +++ b/arch/arm/boot/dts/qcom-mdm9615.dtsi > @@ -28,6 +28,7 @@ cpus { > cpu0: cpu@0 { > compatible = "arm,cortex-a5"; > device_type = "cpu"; > + reg = <0>; Put reg after compatible. > next-level-cache = <&L2>; > }; > }; > Best regards, Krzysztof
On 28/09/2022 11:14, Neil Armstrong wrote: > The spi-max-frequency property has nothing to do in the controller's node, > remove it and fix the 'spi-max-frequency' was unexpected dtbs check error. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On 28/09/2022 11:14, Neil Armstrong wrote: > The PMIC is an PM8018, but was compatible with the PM8921. Both compatibles > was left but it makes no sense anymore the leave both. Why? It makes sense for backwards compatibility. If you think it does not make sense, please say why. > > The pwrkey compatible is left to PM8921, unlike the others because > the interface is stricly compatible with the PM9821 pwrkey. typo: strictly typo: PM8921 Again, why? The old code looked correct. In all three places. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > Best regards, Krzysztof
On 28/09/2022 11:14, Neil Armstrong wrote: > Since amba node type has been deprecated, remove this subnode and How device node can be deprecated? simple-bus is still supported, isn't it? > move the mmc nodes in the main soc node. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> While reshuffling maybe move properties to match common style: 1. compatible 2. reg 3. ... 4. status (last) > Best regards, Krzysztof
On 28/09/2022 20:05, Krzysztof Kozlowski wrote: > On 28/09/2022 11:14, Neil Armstrong wrote: >> Since amba node type has been deprecated, remove this subnode and > > How device node can be deprecated? simple-bus is still supported, isn't it? The amba subnode remained after the amba compatible was changed to simple-bus, there's no need for such subnode anymore. > > >> move the mmc nodes in the main soc node. >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > > While reshuffling maybe move properties to match common style: > 1. compatible > 2. reg > 3. ... > 4. status (last) Ack, will do. > >> > Best regards, > Krzysztof > Thanks, Neil
On 28/09/2022 19:55, Krzysztof Kozlowski wrote: > On 28/09/2022 11:14, Neil Armstrong wrote: >> Replace the licence blob by a clean SPDX-License-Identifier >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> > > > Rebase on linux-next - you use old Bjorn's email address. Yep sorry, my first submission from Linaro... I added Bjorn's new email (+mine) in my local mailmap so it won't happen anymore. > >> diff --git a/arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dts b/arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dts >> index 0827de5426c1..073c15354483 100644 >> --- a/arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dts >> +++ b/arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dts >> @@ -1,46 +1,9 @@ >> +// SPDX-License-Identifier: GPL-2.0+ OR X11 >> /* >> * Device Tree Source for mangOH Green Board with WP8548 Module >> * >> * Copyright (C) 2016 BayLibre, SAS. >> * Author : Neil Armstrong <narmstrong@baylibre.com> >> - * >> - * This file is dual-licensed: you can use it either under the terms >> - * of the GPL or the X11 license, at your option. Note that this dual >> - * licensing only applies to this file, and not this project as a >> - * whole. >> - * >> - * a) This file is free software; you can redistribute it and/or >> - * modify it under the terms of the GNU General Public License as >> - * published by the Free Software Foundation; either version 2 of the >> - * License, or (at your option) any later version. >> - * >> - * This file is distributed in the hope that it will be useful, >> - * but WITHOUT ANY WARRANTY; without even the implied warranty of >> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> - * GNU General Public License for more details. >> - * >> - * Or, alternatively, >> - * >> - * b) Permission is hereby granted, free of charge, to any person >> - * obtaining a copy of this software and associated documentation >> - * files (the "Software"), to deal in the Software without >> - * restriction, including without limitation the rights to use, >> - * copy, modify, merge, publish, distribute, sublicense, and/or >> - * sell copies of the Software, and to permit persons to whom the >> - * Software is furnished to do so, subject to the following >> - * conditions: >> - * >> - * The above copyright notice and this permission notice shall be >> - * included in all copies or substantial portions of the Software. >> - * >> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >> - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES >> - * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND >> - * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT >> - * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, >> - * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING >> - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR >> - * OTHER DEALINGS IN THE SOFTWARE. > > The text is actually MIT, not X11. I think they differ by last X11 > trademark statement. You're right, I checked the amlogic DT SPDX migration that used the same licence blob [1], and we switched to SPDX-License-Identifier: (GPL-2.0+ OR MIT). > > Best regards, > Krzysztof > [1] 114abfe1aa5f ("ARM64: dts: amlogic: Convert to new-style SPDX license identifiers") Thanks, Neil
Hi, On 28/09/2022 20:03, Krzysztof Kozlowski wrote: > On 28/09/2022 11:14, Neil Armstrong wrote: >> The PMIC is an PM8018, but was compatible with the PM8921. Both compatibles >> was left but it makes no sense anymore the leave both. > > Why? It makes sense for backwards compatibility. If you think it does > not make sense, please say why. We had the same debate at submission 7y ago, some of the pm8018 new compatible were rejected in bindings & drivers so I left both... As of today only the pwrkey bindings is missing, so should I resubmit the pm8018-pwrkey bidings and drop the pm8921-pwrkey compatible ? > >> >> The pwrkey compatible is left to PM8921, unlike the others because >> the interface is stricly compatible with the PM9821 pwrkey. > > typo: strictly > typo: PM8921 > > Again, why? The old code looked correct. In all three places. The qcom,pm8018-rtc require a single compatible, same for qcom,pm8018, so what's the way to fix it ? > >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> > > Best regards, > Krzysztof > Thanks, Neil
On 29/09/2022 10:19, Neil Armstrong wrote: > On 28/09/2022 20:05, Krzysztof Kozlowski wrote: > > On 28/09/2022 11:14, Neil Armstrong wrote: > >> Since amba node type has been deprecated, remove this subnode and > > > > How device node can be deprecated? simple-bus is still supported, isn't it? > The amba subnode remained after the amba compatible was changed to simple-bus, there's no need for such subnode anymore. OK, but it is quite different than a node type being deprecated. Best regards, Krzysztof
On 29/09/2022 10:29, Neil Armstrong wrote: > Hi, > > On 28/09/2022 20:03, Krzysztof Kozlowski wrote: >> On 28/09/2022 11:14, Neil Armstrong wrote: >>> The PMIC is an PM8018, but was compatible with the PM8921. Both compatibles >>> was left but it makes no sense anymore the leave both. >> >> Why? It makes sense for backwards compatibility. If you think it does >> not make sense, please say why. > > We had the same debate at submission 7y ago, some of the pm8018 new compatible > were rejected in bindings & drivers so I left both... > > As of today only the pwrkey bindings is missing, so should I resubmit the pm8018-pwrkey bidings and > drop the pm8921-pwrkey compatible ? ~7 years ago here: https://lore.kernel.org/all/20160624220748.GB11719@dtor-ws/ you proposed to add something entirely different than we have here now and than we talk about. In that thread you correctly wrote: "My point of view is that the devicetree describes the hardware and need to have SoC specific compatible string since it describes the actual silicon, and drivers must make sure to handle all the SoC or family variants using the compatible string and the match data." but implemented it entirely different. Maybe you refer to different mail thread, I don't know, but that one is indeed wrong. The DTS looks correct unless you have some real argument that it is not. How this should be fixed? First, drop bogus entries from drivers, then document proper compatibles. Best regards, Krzysztof
On 29/09/2022 11:12, Krzysztof Kozlowski wrote: > On 29/09/2022 10:29, Neil Armstrong wrote: >> Hi, >> >> On 28/09/2022 20:03, Krzysztof Kozlowski wrote: >>> On 28/09/2022 11:14, Neil Armstrong wrote: >>>> The PMIC is an PM8018, but was compatible with the PM8921. Both compatibles >>>> was left but it makes no sense anymore the leave both. >>> >>> Why? It makes sense for backwards compatibility. If you think it does >>> not make sense, please say why. >> >> We had the same debate at submission 7y ago, some of the pm8018 new compatible >> were rejected in bindings & drivers so I left both... >> >> As of today only the pwrkey bindings is missing, so should I resubmit the pm8018-pwrkey bidings and >> drop the pm8921-pwrkey compatible ? > > ~7 years ago here: > https://lore.kernel.org/all/20160624220748.GB11719@dtor-ws/ > you proposed to add something entirely different than we have here now > and than we talk about. > > In that thread you correctly wrote: > "My point of view is that the devicetree describes the hardware and need > to have SoC specific compatible string since it describes the actual > silicon, and drivers must make sure to handle all the SoC or family > variants using the compatible string and the match data." And I'm happy this is still the policy! And I'm tried my best to follow this in all my DT & bindings submissions, while DT-Schema helped a lot here. > > but implemented it entirely different. Maybe you refer to different mail > thread, I don't know, but that one is indeed wrong. In the meantime things got much better, but at that time pushing a SoC bringup was a pain (I did 2 at the time, the other one is the OX810SE) and I even mentioned it in a talk ([1] slides 27 to 30). So I added both to be sure that at some point a driver would probe against one of the compatible entries... > > The DTS looks correct unless you have some real argument that it is not. > > How this should be fixed? First, drop bogus entries from drivers, then > document proper compatibles. What do you mean ? There's no point to keep the PM8921 compatibles, the gpio and PMIC bindings already enforces to only have the PM8018 compatible. The only issue is about the PM8018 pwrkey, where the solution would be to actually re-submit [1] by documenting qcom,pm8018-pwrkey and adding the entry in the drivers/input/misc/pmic8xxx-pwrkey.c driver. Or maybe I missed something. [1] https://www.slideshare.net/superna/elce-2016-neil-armstrong-no-its-never-too-late-to-upstream-your-legacy-linux-based-platform [2] https://lore.kernel.org/all/1466759887-25394-3-git-send-email-narmstrong@baylibre.com/ > > Best regards, > Krzysztof > Thanks, Neil
On 29/09/2022 11:05, Krzysztof Kozlowski wrote: > On 29/09/2022 10:19, Neil Armstrong wrote: >> On 28/09/2022 20:05, Krzysztof Kozlowski wrote: >> > On 28/09/2022 11:14, Neil Armstrong wrote: >> >> Since amba node type has been deprecated, remove this subnode and >> > >> > How device node can be deprecated? simple-bus is still supported, isn't it? >> The amba subnode remained after the amba compatible was changed to simple-bus, there's no need for such subnode anymore. > > OK, but it is quite different than a node type being deprecated. Well it's still related, it's because of the deprecation we have a now useless subnode, anyway will re-word for v2. > > Best regards, > Krzysztof > Thanks, Neil
On 29/09/2022 12:56, Neil Armstrong wrote: > On 29/09/2022 11:12, Krzysztof Kozlowski wrote: >> On 29/09/2022 10:29, Neil Armstrong wrote: >>> Hi, >>> >>> On 28/09/2022 20:03, Krzysztof Kozlowski wrote: >>>> On 28/09/2022 11:14, Neil Armstrong wrote: >>>>> The PMIC is an PM8018, but was compatible with the PM8921. Both compatibles >>>>> was left but it makes no sense anymore the leave both. >>>> >>>> Why? It makes sense for backwards compatibility. If you think it does >>>> not make sense, please say why. >>> >>> We had the same debate at submission 7y ago, some of the pm8018 new compatible >>> were rejected in bindings & drivers so I left both... >>> >>> As of today only the pwrkey bindings is missing, so should I resubmit the pm8018-pwrkey bidings and >>> drop the pm8921-pwrkey compatible ? >> >> ~7 years ago here: >> https://lore.kernel.org/all/20160624220748.GB11719@dtor-ws/ >> you proposed to add something entirely different than we have here now >> and than we talk about. >> >> In that thread you correctly wrote: >> "My point of view is that the devicetree describes the hardware and need >> to have SoC specific compatible string since it describes the actual >> silicon, and drivers must make sure to handle all the SoC or family >> variants using the compatible string and the match data." > > And I'm happy this is still the policy! And I'm tried my best to follow this > in all my DT & bindings submissions, while DT-Schema helped a lot here. > >> >> but implemented it entirely different. Maybe you refer to different mail >> thread, I don't know, but that one is indeed wrong. > > In the meantime things got much better, but at that time pushing a SoC bringup > was a pain (I did 2 at the time, the other one is the OX810SE) and I even > mentioned it in a talk ([1] slides 27 to 30). > > So I added both to be sure that at some point a driver would probe against > one of the compatible entries... > >> >> The DTS looks correct unless you have some real argument that it is not. >> >> How this should be fixed? First, drop bogus entries from drivers, then >> document proper compatibles. > > What do you mean ? There's no point to keep the PM8921 compatibles, the gpio I asked at beginning - why? Why there is no point to keep them? > and PMIC bindings already enforces to only have the PM8018 compatible. That is just partial argument because binding does not match DTS. So something is not correct. Why do you assume bindings are correct? > > The only issue is about the PM8018 pwrkey, where the solution would be > to actually re-submit [1] by documenting qcom,pm8018-pwrkey and adding the entry > in the drivers/input/misc/pmic8xxx-pwrkey.c driver. > > Or maybe I missed something. > > [1] https://www.slideshare.net/superna/elce-2016-neil-armstrong-no-its-never-too-late-to-upstream-your-legacy-linux-based-platform > [2] https://lore.kernel.org/all/1466759887-25394-3-git-send-email-narmstrong@baylibre.com/ So let's repeat again: the patch [2] looks wrong. The qcom,pm8018-pwrkey and qcom,pm8921-pwrkey are compatible. Best regards, Krzysztof
On 29/09/2022 13:12, Krzysztof Kozlowski wrote: > On 29/09/2022 12:56, Neil Armstrong wrote: >> On 29/09/2022 11:12, Krzysztof Kozlowski wrote: >>> On 29/09/2022 10:29, Neil Armstrong wrote: >>>> Hi, >>>> >>>> On 28/09/2022 20:03, Krzysztof Kozlowski wrote: >>>>> On 28/09/2022 11:14, Neil Armstrong wrote: >>>>>> The PMIC is an PM8018, but was compatible with the PM8921. Both compatibles >>>>>> was left but it makes no sense anymore the leave both. >>>>> >>>>> Why? It makes sense for backwards compatibility. If you think it does >>>>> not make sense, please say why. >>>> >>>> We had the same debate at submission 7y ago, some of the pm8018 new compatible >>>> were rejected in bindings & drivers so I left both... >>>> >>>> As of today only the pwrkey bindings is missing, so should I resubmit the pm8018-pwrkey bidings and >>>> drop the pm8921-pwrkey compatible ? >>> >>> ~7 years ago here: >>> https://lore.kernel.org/all/20160624220748.GB11719@dtor-ws/ >>> you proposed to add something entirely different than we have here now >>> and than we talk about. >>> >>> In that thread you correctly wrote: >>> "My point of view is that the devicetree describes the hardware and need >>> to have SoC specific compatible string since it describes the actual >>> silicon, and drivers must make sure to handle all the SoC or family >>> variants using the compatible string and the match data." >> >> And I'm happy this is still the policy! And I'm tried my best to follow this >> in all my DT & bindings submissions, while DT-Schema helped a lot here. >> >>> >>> but implemented it entirely different. Maybe you refer to different mail >>> thread, I don't know, but that one is indeed wrong. >> >> In the meantime things got much better, but at that time pushing a SoC bringup >> was a pain (I did 2 at the time, the other one is the OX810SE) and I even >> mentioned it in a talk ([1] slides 27 to 30). >> >> So I added both to be sure that at some point a driver would probe against >> one of the compatible entries... >> >>> >>> The DTS looks correct unless you have some real argument that it is not. >>> >>> How this should be fixed? First, drop bogus entries from drivers, then >>> document proper compatibles. >> >> What do you mean ? There's no point to keep the PM8921 compatibles, the gpio > > I asked at beginning - why? Why there is no point to keep them? Because the HW is an PM8018 and the addition of the PM8921 was for policy/organization/struggling-to-make-dt-merged-before-clear-dt-policy/... so you say I should modify the Bindings to reflect the actual "pm8018", "pm8921" situation instead of changing the DT even if incorrect ? > >> and PMIC bindings already enforces to only have the PM8018 compatible. > > That is just partial argument because binding does not match DTS. So > something is not correct. Why do you assume bindings are correct? Because bindings accurately reflects HW and DT doesn't. > >> >> The only issue is about the PM8018 pwrkey, where the solution would be >> to actually re-submit [1] by documenting qcom,pm8018-pwrkey and adding the entry >> in the drivers/input/misc/pmic8xxx-pwrkey.c driver. >> >> Or maybe I missed something. >> >> [1] https://www.slideshare.net/superna/elce-2016-neil-armstrong-no-its-never-too-late-to-upstream-your-legacy-linux-based-platform >> [2] https://lore.kernel.org/all/1466759887-25394-3-git-send-email-narmstrong@baylibre.com/ > > So let's repeat again: the patch [2] looks wrong. The qcom,pm8018-pwrkey > and qcom,pm8921-pwrkey are compatible. Ok, I need time to understand, I'm highly confused now. > > Best regards, > Krzysztof >
On 29/09/2022 13:39, Neil Armstrong wrote: >>>> The DTS looks correct unless you have some real argument that it is not. >>>> >>>> How this should be fixed? First, drop bogus entries from drivers, then >>>> document proper compatibles. >>> >>> What do you mean ? There's no point to keep the PM8921 compatibles, the gpio >> >> I asked at beginning - why? Why there is no point to keep them? > > Because the HW is an PM8018 and the addition of the PM8921 was for policy/organization/struggling-to-make-dt-merged-before-clear-dt-policy/... > so you say I should modify the Bindings to reflect the actual "pm8018", "pm8921" situation instead of changing the DT even if incorrect ? Yes, this is what I already wrote: "How this should be fixed? First, drop bogus entries from drivers, then document proper compatibles." >>> and PMIC bindings already enforces to only have the PM8018 compatible. >> >> That is just partial argument because binding does not match DTS. So >> something is not correct. Why do you assume bindings are correct? > > Because bindings accurately reflects HW and DT doesn't. That's not really an answer... Bindings are correct because they are correct? What is exactly correct in the bindings? How they reflect the HW in a proper way, while DTS does not? Or let's focus on actual hardware - what are the properties of the hardware which indicate that DTS is wrong? Best regards, Krzysztof
On 29/09/2022 13:47, Krzysztof Kozlowski wrote: > On 29/09/2022 13:39, Neil Armstrong wrote: >>>>> The DTS looks correct unless you have some real argument that it is not. >>>>> >>>>> How this should be fixed? First, drop bogus entries from drivers, then >>>>> document proper compatibles. >>>> >>>> What do you mean ? There's no point to keep the PM8921 compatibles, the gpio >>> >>> I asked at beginning - why? Why there is no point to keep them? >> >> Because the HW is an PM8018 and the addition of the PM8921 was for policy/organization/struggling-to-make-dt-merged-before-clear-dt-policy/... >> so you say I should modify the Bindings to reflect the actual "pm8018", "pm8921" situation instead of changing the DT even if incorrect ? > > Yes, this is what I already wrote: > > "How this should be fixed? First, drop bogus entries from drivers, then > document proper compatibles." > >>>> and PMIC bindings already enforces to only have the PM8018 compatible. >>> >>> That is just partial argument because binding does not match DTS. So >>> something is not correct. Why do you assume bindings are correct? >> >> Because bindings accurately reflects HW and DT doesn't. > > That's not really an answer... Bindings are correct because they are > correct? What is exactly correct in the bindings? How they reflect the > HW in a proper way, while DTS does not? > > Or let's focus on actual hardware - what are the properties of the > hardware which indicate that DTS is wrong? The actual PMIC is an PM8018 > > Best regards, > Krzysztof > Neil
On 29/09/2022 13:59, Neil Armstrong wrote: >> That's not really an answer... Bindings are correct because they are >> correct? What is exactly correct in the bindings? How they reflect the >> HW in a proper way, while DTS does not? >> >> Or let's focus on actual hardware - what are the properties of the >> hardware which indicate that DTS is wrong? > > The actual PMIC is an PM8018 And DTS is saying PMIC is PM8018, isn't it? I see clearly in DTS: qcom,pm8018 qcom,pm8018-rtc qcom,pm8018-pwrkey qcom,pm8018-gpio Best regards, Krzysztof
On 29/09/2022 14:02, Krzysztof Kozlowski wrote: > On 29/09/2022 13:59, Neil Armstrong wrote: >>> That's not really an answer... Bindings are correct because they are >>> correct? What is exactly correct in the bindings? How they reflect the >>> HW in a proper way, while DTS does not? >>> >>> Or let's focus on actual hardware - what are the properties of the >>> hardware which indicate that DTS is wrong? >> >> The actual PMIC is an PM8018 > > And DTS is saying PMIC is PM8018, isn't it? I see clearly in DTS: > qcom,pm8018 > qcom,pm8018-rtc > qcom,pm8018-pwrkey > qcom,pm8018-gpio And this is why I pushed the removal of qcom,pm8921* fallback compatibles, except for qcom,pm8018-pwrkey because I didn't managed to get it documented at the time. > > Best regards, > Krzysztof >
On 29/09/2022 14:21, Neil Armstrong wrote: > On 29/09/2022 14:02, Krzysztof Kozlowski wrote: >> On 29/09/2022 13:59, Neil Armstrong wrote: >>>> That's not really an answer... Bindings are correct because they are >>>> correct? What is exactly correct in the bindings? How they reflect the >>>> HW in a proper way, while DTS does not? >>>> >>>> Or let's focus on actual hardware - what are the properties of the >>>> hardware which indicate that DTS is wrong? >>> >>> The actual PMIC is an PM8018 >> >> And DTS is saying PMIC is PM8018, isn't it? I see clearly in DTS: >> qcom,pm8018 >> qcom,pm8018-rtc >> qcom,pm8018-pwrkey >> qcom,pm8018-gpio > > And this is why I pushed the removal of qcom,pm8921* fallback compatibles, > except for qcom,pm8018-pwrkey because I didn't managed to get it documented at the time. This does not explain at all why you wanted to remove any other compatibles. There is no connection, relation between these. We are making circles and discussion takes too much. I asked to bring the arguments about hardware that point devices are not compatible. You just said "PMIC is an PM8018", and that's it. Nothing more, nothing about hardware. Based on that you want to remove compatibility. This is not valid argument. It's unrelated. You could as well say "The actual PMIC is Qualcomm PMIC" and you would be right. Still not an argument. Based on lack of arguments in this entire discussion, the patch itself is not correct. Use the approach I wrote some time ago and quoted one more time. Best regards, Krzysztof
Hi, On 29/09/2022 14:27, Krzysztof Kozlowski wrote: > > We are making circles and discussion takes too much. I'm sorry this happens, but I really want solve this stuff which in suspend since 2015. So let me recall the original issue: DTBS check reports: arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dtb: pmic@0: compatible: ['qcom,pm8018', 'qcom,pm8921'] is too long From schema: Documentation/devicetree/bindings/mfd/qcom-pm8xxx.yaml arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dtb: pmic@0: rtc@11d:compatible: ['qcom,pm8018-rtc', 'qcom,pm8921-rtc'] is too long From schema: Documentation/devicetree/bindings/mfd/qcom-pm8xxx.yaml arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dtb:0:0: /soc/qcom,ssbi@500000/pmic@0/pwrkey@1c: failed to match any schema with compatible: ['qcom,pm8018-pwrkey', 'qcom,pm8921-pwrkey'] arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dtb:0:0: /soc/qcom,ssbi@500000/pmic@0/pwrkey@1c: failed to match any schema with compatible: ['qcom,pm8018-pwrkey', 'qcom,pm8921-pwrkey'] arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dtb: rtc@11d: compatible: ['qcom,pm8018-rtc', 'qcom,pm8921-rtc'] is too long From schema: Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml So trying to solve those, and since the PMIC in the wp8548 module is a PM8018, and it happens to be (partially ?? potentially ??) compatible with the PM8921, and I had issues adding per-HW compatible for the pwrkey, the obvious solution would be to drop the PM8921 compatibility since it's only probable and nothing proves it's right. But what's sure: it's a PM8018 PMIC. But since the PM8018 PWRKEY interface is compatible with the PM8921 PWRKEY interface, it's perfectly ok to the the MP8921 compatible here. OK, so as you quoted multiple times: "How this should be fixed? First, drop bogus entries from drivers, then document proper compatibles." OK so there's no bogus entries to remove here, and the only compatible to potentially document is the pm8018-pwrkey but it seems to be wrong. I'll be happy to have an hint on how to handle that to I can go forward and stop the noise, there's still plenty of stuff to fix in the MDM9615 DT. > > Best regards, > Krzysztof > Thanks, Neil
On 29/09/2022 14:48, Neil Armstrong wrote: > Hi, > > On 29/09/2022 14:27, Krzysztof Kozlowski wrote: >> >> We are making circles and discussion takes too much. > > I'm sorry this happens, but I really want solve this stuff which in suspend since 2015. > > So let me recall the original issue: > > DTBS check reports: > > arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dtb: pmic@0: compatible: ['qcom,pm8018', 'qcom,pm8921'] is too long > From schema: Documentation/devicetree/bindings/mfd/qcom-pm8xxx.yaml > > arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dtb: pmic@0: rtc@11d:compatible: ['qcom,pm8018-rtc', 'qcom,pm8921-rtc'] is too long > From schema: Documentation/devicetree/bindings/mfd/qcom-pm8xxx.yaml > > arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dtb:0:0: /soc/qcom,ssbi@500000/pmic@0/pwrkey@1c: failed to match any schema with compatible: ['qcom,pm8018-pwrkey', 'qcom,pm8921-pwrkey'] > > arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dtb:0:0: /soc/qcom,ssbi@500000/pmic@0/pwrkey@1c: failed to match any schema with compatible: ['qcom,pm8018-pwrkey', 'qcom,pm8921-pwrkey'] > > arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dtb: rtc@11d: compatible: ['qcom,pm8018-rtc', 'qcom,pm8921-rtc'] is too long > From schema: Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml > > So trying to solve those, and since the PMIC in the wp8548 module is a PM8018, and it happens to be (partially ?? potentially ??) compatible > with the PM8921, and I had issues adding per-HW compatible for the pwrkey, the obvious solution would be to > drop the PM8921 compatibility since it's only probable and nothing proves it's right. Although it is obvious solution it is also affecting all out-of-tree users of DTS. > > But what's sure: it's a PM8018 PMIC. I could not find the spec for both of these, but similar numbers have for example exactly the same RTC. I guess other blocks are also the same. > > But since the PM8018 PWRKEY interface is compatible with the PM8921 PWRKEY interface, > it's perfectly ok to the the MP8921 compatible here. > > OK, so as you quoted multiple times: > "How this should be fixed? First, drop bogus entries from drivers, then > document proper compatibles." > > OK so there's no bogus entries to remove here, and the only compatible to > potentially document is the pm8018-pwrkey but it seems to be wrong. All the entries in drivers which are duplicating the fallback are not needed. I called them bogus because adding them brought no meaning. > I'll be happy to have an hint on how to handle that to I can go forward and > stop the noise, there's still plenty of stuff to fix in the MDM9615 DT. Drop the unneeded entries from the driver, document (properly) the compatible how it is used in DTS (so not in some other way than DTS expresses it). Best regards, Krzysztof
This is a first round of trivial bindings & DT fixes for the MDM9615 platform. This first round focuses on trivial changes, the remaining work will mainly be .txt to .yaml transition of old qcom pmic & co device bindings. To: Andy Gross <agross@kernel.org> To: Bjorn Andersson <bjorn.andersson@linaro.org> To: Konrad Dybcio <konrad.dybcio@somainline.org> To: Rob Herring <robh+dt@kernel.org> To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> Cc: linux-arm-msm@vger.kernel.org Cc: devicetree@vger.kernel.org Cc: linux-kernel@vger.kernel.org Dependencies: None Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- Neil Armstrong (7): dt-bindings: arm: qcom: document the swir,mangoh-green-wp8548 board arm: dts: qcom: mdm9615*: add SPDX-License-Identifier arm: dts: qcom: mdm9615: add missing reg in cpu@0 node arm: dts: qcom: mdm9615: remove invalid spi-max-frequency gsbi3_spi node arm: dts: qcom: mdm9615: remove invalid pmic subnodes compatibles arm: dts: qcom: mdm9615: remove invalid interrupt-names from pl18x mmc nodes arm: dts: qcom: mdm9615: remove useless amba subnode Documentation/devicetree/bindings/arm/qcom.yaml | 6 + .../boot/dts/qcom-mdm9615-wp8548-mangoh-green.dts | 39 +------ arch/arm/boot/dts/qcom-mdm9615-wp8548.dtsi | 39 +------ arch/arm/boot/dts/qcom-mdm9615.dtsi | 127 +++++++-------------- 4 files changed, 49 insertions(+), 162 deletions(-) --- base-commit: f76349cf41451c5c42a99f18a9163377e4b364ff change-id: 20220928-mdm9615-dt-schema-fixes-66d4d0ccb7c7 Best regards,