| Message ID | 20231109-amlogic-v6-4-upstream-dsi-ccf-vim3-v8-4-81e4aeeda193@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/meson: add support for MIPI DSI Display | expand |
On Thu, Nov 09, 2023 at 10:00:05AM +0100, Neil Armstrong wrote: > Now this bindings is referred from amlogic,meson-gx-hhi-sysctrl.yaml and is > documented as a subnode of a simple-mfd, drop the invalid reg property. I'd expect a note here tbh about how removing reg & relying on being a subnode of the simple-mfd is safe to do. It looks like your driver was added at the same time as this binding & it was always documented as being a child of the simple-mfd system controller, so I'd kinda expect to see a Fixes tag on this patch.. Am I missing something? > > Also drop the unnecessary example, the top level bindings example should > be enough. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > .../bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml | 12 ------------ > 1 file changed, 12 deletions(-) > > diff --git a/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml b/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml > index c8c83acfb871..81c2654b7e57 100644 > --- a/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml > +++ b/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml > @@ -16,20 +16,8 @@ properties: > "#phy-cells": > const: 0 > > - reg: > - maxItems: 1 > - > required: > - compatible > - - reg > - "#phy-cells" > > additionalProperties: false > - > -examples: > - - | > - phy@0 { > - compatible = "amlogic,g12a-mipi-dphy-analog"; > - reg = <0x0 0xc>; > - #phy-cells = <0>; > - }; > > -- > 2.34.1 >
On 09/11/2023 19:04, Conor Dooley wrote: > On Thu, Nov 09, 2023 at 10:00:05AM +0100, Neil Armstrong wrote: >> Now this bindings is referred from amlogic,meson-gx-hhi-sysctrl.yaml and is >> documented as a subnode of a simple-mfd, drop the invalid reg property. > > I'd expect a note here tbh about how removing reg & relying on being a > subnode of the simple-mfd is safe to do. It looks like your driver > was added at the same time as this binding & it was always documented as > being a child of the simple-mfd system controller, so I'd kinda expect > to see a Fixes tag on this patch.. > > Am I missing something? No you're totally right, I'll reword the commit message and add a Fixes tags. Thanks, Neil > >> >> Also drop the unnecessary example, the top level bindings example should >> be enough. >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> .../bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml | 12 ------------ >> 1 file changed, 12 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml b/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml >> index c8c83acfb871..81c2654b7e57 100644 >> --- a/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml >> +++ b/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml >> @@ -16,20 +16,8 @@ properties: >> "#phy-cells": >> const: 0 >> >> - reg: >> - maxItems: 1 >> - >> required: >> - compatible >> - - reg >> - "#phy-cells" >> >> additionalProperties: false >> - >> -examples: >> - - | >> - phy@0 { >> - compatible = "amlogic,g12a-mipi-dphy-analog"; >> - reg = <0x0 0xc>; >> - #phy-cells = <0>; >> - }; >> >> -- >> 2.34.1 >>
On Thu, Nov 09, 2023 at 10:00:05AM +0100, Neil Armstrong wrote: > Now this bindings is referred from amlogic,meson-gx-hhi-sysctrl.yaml and is > documented as a subnode of a simple-mfd, drop the invalid reg property. Why is it invalid? It's preferred to have 'reg' in MFDs even if Linux doesn't use them. If there's a chunk of registers you can define, then do so. If it's all register bit soup, then fine, omit it. Rob
On 10/11/2023 21:57, Rob Herring wrote: > On Thu, Nov 09, 2023 at 10:00:05AM +0100, Neil Armstrong wrote: >> Now this bindings is referred from amlogic,meson-gx-hhi-sysctrl.yaml and is >> documented as a subnode of a simple-mfd, drop the invalid reg property. > > Why is it invalid? It's preferred to have 'reg' in MFDs even if Linux > doesn't use them. If there's a chunk of registers you can define, then > do so. If it's all register bit soup, then fine, omit it. I still don't understand why this particular MFD subnode needs a reg and not the other ones, using reg would need adding #address-cells/#size-cells on top node and change all examples/DT for nothing. Like the other meson-gx-hhi-sysctrl subnodes, it's a register bit soup and this one is no exception. Neil > > Rob
diff --git a/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml b/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml index c8c83acfb871..81c2654b7e57 100644 --- a/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml +++ b/Documentation/devicetree/bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml @@ -16,20 +16,8 @@ properties: "#phy-cells": const: 0 - reg: - maxItems: 1 - required: - compatible - - reg - "#phy-cells" additionalProperties: false - -examples: - - | - phy@0 { - compatible = "amlogic,g12a-mipi-dphy-analog"; - reg = <0x0 0xc>; - #phy-cells = <0>; - };
Now this bindings is referred from amlogic,meson-gx-hhi-sysctrl.yaml and is documented as a subnode of a simple-mfd, drop the invalid reg property. Also drop the unnecessary example, the top level bindings example should be enough. Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- .../bindings/phy/amlogic,g12a-mipi-dphy-analog.yaml | 12 ------------ 1 file changed, 12 deletions(-)