| Message ID | 20220617222916.2435618-3-nfraprado@collabora.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fixes for dtbs_check warnings on Mediatek XHCI nodes | expand |
On Fri, 2022-06-17 at 18:25 -0700, Krzysztof Kozlowski wrote: > On 17/06/2022 15:29, Nícolas F. R. A. Prado wrote: > > The current clock list in the binding doesn't allow for one of the > > optional clocks to be missing and a subsequent clock to be present. > > An > > example where this is an issue is in mt8192.dtsi, which has > > "sys_ck", > > "ref_ck", "xhci_ck" and would cause dtbs_check warnings. > > > > Change the clock list in a way that allows the middle optional > > clocks to > > be missing, while still guaranteeing a fixed order. The "ref_ck" is > > kept > > as a const even though it is optional for simplicity, since it is > > present in all current dts files. > > > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > --- > > > > .../devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 9 > > +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk- > > xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk- > > xhci.yaml > > index 63cbc2b62d18..99a1b233ec90 100644 > > --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml > > +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml > > @@ -80,8 +80,13 @@ properties: > > items: > > - const: sys_ck # required, the following ones are optional > > - const: ref_ck > > - - const: mcu_ck > > - - const: dma_ck > > + - enum: > > + - mcu_ck > > + - dma_ck > > + - xhci_ck > > + - enum: > > + - dma_ck > > + - xhci_ck > > - const: xhci_ck > > You allow now almost any order here, including incorrect like > sys,ref,xhci,xhci,xhci. > > The order of clocks has to be fixed and we cannot allow flexibility. > Are > you sure that these clocks are actually optional (not wired to the > device)? In fact, these optional clocks are fixed, due to no gates are provided, SW can't control them by CCF; In this case, I usually use a fixed clock, or ignore it. > > > Best regards, > Krzysztof
On 19/06/2022 09:40, Chunfeng Yun wrote: > On Fri, 2022-06-17 at 18:29 -0400, Nícolas F. R. A. Prado wrote: >> The current clock list in the binding doesn't allow for one of the >> optional clocks to be missing and a subsequent clock to be present. >> An >> example where this is an issue is in mt8192.dtsi, which has "sys_ck", >> "ref_ck", "xhci_ck" and would cause dtbs_check warnings. > How about using fixed clock instead to fix the check warning? > Using enum way seems make it more complex. > That would mean the clock is not actually optional. The DTS should reflect the hardware so either you have the clock there or not. Either it is an input or not. Of course there are some exceptions (like non-controllable clock or regulator which not always has to be modeled). Best regards, Krzysztof
On Sun, 2022-06-19 at 14:05 +0200, Krzysztof Kozlowski wrote: > On 19/06/2022 09:46, Chunfeng Yun wrote: > > On Fri, 2022-06-17 at 18:25 -0700, Krzysztof Kozlowski wrote: > > > On 17/06/2022 15:29, Nícolas F. R. A. Prado wrote: > > > > The current clock list in the binding doesn't allow for one of > > > > the > > > > optional clocks to be missing and a subsequent clock to be > > > > present. > > > > An > > > > example where this is an issue is in mt8192.dtsi, which has > > > > "sys_ck", > > > > "ref_ck", "xhci_ck" and would cause dtbs_check warnings. > > > > > > > > Change the clock list in a way that allows the middle optional > > > > clocks to > > > > be missing, while still guaranteeing a fixed order. The > > > > "ref_ck" is > > > > kept > > > > as a const even though it is optional for simplicity, since it > > > > is > > > > present in all current dts files. > > > > > > > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > > > --- > > > > > > > > .../devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 9 > > > > +++++++-- > > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/usb/mediatek,mtk- > > > > xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk- > > > > xhci.yaml > > > > index 63cbc2b62d18..99a1b233ec90 100644 > > > > --- a/Documentation/devicetree/bindings/usb/mediatek,mtk- > > > > xhci.yaml > > > > +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk- > > > > xhci.yaml > > > > @@ -80,8 +80,13 @@ properties: > > > > items: > > > > - const: sys_ck # required, the following ones are > > > > optional > > > > - const: ref_ck > > > > - - const: mcu_ck > > > > - - const: dma_ck > > > > + - enum: > > > > + - mcu_ck > > > > + - dma_ck > > > > + - xhci_ck > > > > + - enum: > > > > + - dma_ck > > > > + - xhci_ck > > > > - const: xhci_ck > > > > > > You allow now almost any order here, including incorrect like > > > sys,ref,xhci,xhci,xhci. > > > > > > The order of clocks has to be fixed and we cannot allow > > > flexibility. > > > Are > > > you sure that these clocks are actually optional (not wired to > > > the > > > device)? > > > > In fact, these optional clocks are fixed, due to no gates are > > provided, > > SW can't control them by CCF; > > In this case, I usually use a fixed clock, or ignore it. > > But in some versions these clocks are controllable or not? Some SoCs are controllable, some ones are not (fixed clock). > > Best regards, > Krzysztof
On 20/06/2022 08:59, Chunfeng Yun wrote: > On Sun, 2022-06-19 at 14:05 +0200, Krzysztof Kozlowski wrote: >> On 19/06/2022 09:46, Chunfeng Yun wrote: >>> On Fri, 2022-06-17 at 18:25 -0700, Krzysztof Kozlowski wrote: >>>> On 17/06/2022 15:29, Nícolas F. R. A. Prado wrote: >>>>> The current clock list in the binding doesn't allow for one of >>>>> the >>>>> optional clocks to be missing and a subsequent clock to be >>>>> present. >>>>> An >>>>> example where this is an issue is in mt8192.dtsi, which has >>>>> "sys_ck", >>>>> "ref_ck", "xhci_ck" and would cause dtbs_check warnings. >>>>> >>>>> Change the clock list in a way that allows the middle optional >>>>> clocks to >>>>> be missing, while still guaranteeing a fixed order. The >>>>> "ref_ck" is >>>>> kept >>>>> as a const even though it is optional for simplicity, since it >>>>> is >>>>> present in all current dts files. >>>>> >>>>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> >>>>> --- >>>>> >>>>> .../devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 9 >>>>> +++++++-- >>>>> 1 file changed, 7 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git >>>>> a/Documentation/devicetree/bindings/usb/mediatek,mtk- >>>>> xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk- >>>>> xhci.yaml >>>>> index 63cbc2b62d18..99a1b233ec90 100644 >>>>> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk- >>>>> xhci.yaml >>>>> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk- >>>>> xhci.yaml >>>>> @@ -80,8 +80,13 @@ properties: >>>>> items: >>>>> - const: sys_ck # required, the following ones are >>>>> optional >>>>> - const: ref_ck >>>>> - - const: mcu_ck >>>>> - - const: dma_ck >>>>> + - enum: >>>>> + - mcu_ck >>>>> + - dma_ck >>>>> + - xhci_ck >>>>> + - enum: >>>>> + - dma_ck >>>>> + - xhci_ck >>>>> - const: xhci_ck >>>> >>>> You allow now almost any order here, including incorrect like >>>> sys,ref,xhci,xhci,xhci. >>>> >>>> The order of clocks has to be fixed and we cannot allow >>>> flexibility. >>>> Are >>>> you sure that these clocks are actually optional (not wired to >>>> the >>>> device)? >>> >>> In fact, these optional clocks are fixed, due to no gates are >>> provided, >>> SW can't control them by CCF; >>> In this case, I usually use a fixed clock, or ignore it. >> >> But in some versions these clocks are controllable or not? > Some SoCs are controllable, some ones are not (fixed clock). Thanks for confirming. Then I would prefer to make these clocks required (not optional) and always provide them - via common clock framework or fixed-clock. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml index 63cbc2b62d18..99a1b233ec90 100644 --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml @@ -80,8 +80,13 @@ properties: items: - const: sys_ck # required, the following ones are optional - const: ref_ck - - const: mcu_ck - - const: dma_ck + - enum: + - mcu_ck + - dma_ck + - xhci_ck + - enum: + - dma_ck + - xhci_ck - const: xhci_ck assigned-clocks:
The current clock list in the binding doesn't allow for one of the optional clocks to be missing and a subsequent clock to be present. An example where this is an issue is in mt8192.dtsi, which has "sys_ck", "ref_ck", "xhci_ck" and would cause dtbs_check warnings. Change the clock list in a way that allows the middle optional clocks to be missing, while still guaranteeing a fixed order. The "ref_ck" is kept as a const even though it is optional for simplicity, since it is present in all current dts files. Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> --- .../devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)