On Thu, Jun 22, 2023 at 8:12 AM Conor Dooley <conor.dooley@xxxxxxxxxxxxx> wrote: > > On Thu, Jun 22, 2023 at 07:43:31AM +0100, Lucas Tanure wrote: > > On Thu, Jun 22, 2023 at 7:05 AM Krzysztof Kozlowski > > <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > > On 22/06/2023 07:32, Lucas Tanure wrote: > > > > On Wed, Jun 21, 2023 at 7:12 PM Conor Dooley <conor@xxxxxxxxxx> wrote: > > > >> On Wed, Jun 21, 2023 at 03:53:04PM +0200, Krzysztof Kozlowski wrote: > > > >>> On 21/06/2023 15:32, Lucas Tanure wrote: > > > >>>> Amlogic T7 SoCs uses the same UART controller as S4 SoCs and G12A. > > > >>>> There is no need for an extra compatible line in the driver, but > > > >>>> add T7 compatible line for documentation. > > > >>>> > > > >>>> Signed-off-by: Lucas Tanure <tanure@xxxxxxxxx> > > > >>>> --- > > > >>>> .../devicetree/bindings/serial/amlogic,meson-uart.yaml | 2 ++ > > > >>>> 1 file changed, 2 insertions(+) > > > >>>> > > > >>>> diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml > > > >>>> index 01ec45b3b406..860ab58d87b0 100644 > > > >>>> --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml > > > >>>> +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml > > > >>>> @@ -33,6 +33,7 @@ properties: > > > >>>> - amlogic,meson8b-uart > > > >>>> - amlogic,meson-gx-uart > > > >>>> - amlogic,meson-s4-uart > > > >>>> + - amlogic,meson-t7-uart > > > >>>> - const: amlogic,meson-ao-uart > > > >>>> - description: Always-on power domain UART controller on G12A SoCs > > > >>>> items: > > > >>>> @@ -46,6 +47,7 @@ properties: > > > >>>> - amlogic,meson8b-uart > > > >>>> - amlogic,meson-gx-uart > > > >>>> - amlogic,meson-s4-uart > > > >>>> + - amlogic,meson-t7-uart > > > >>> > > > >>> It does not look like you tested the DTS against bindings. Please run > > > >>> `make dtbs_check` (see > > > >>> Documentation/devicetree/bindings/writing-schema.rst or > > > >>> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ > > > >>> for instructions). > > > >> > > > >> Check back on the previous version, I should've posted an untested > > > >> version of what you need to add. > > > > I saw that, but adding a S4 doesn't make sense to me. And you didn't > > > > show the entire change, so I can't understand what you want there. > > > > > > For sure you need something which does not trigger errors. If you claim > > > adding S4 as fallback does not make sense, then why did you use it? > > > Sending a code which is clearly incorrect does not make sense. > > > > > Sorry, I think we are talking about different things. It does not make > > sense to me to add an S4 line in the documentation when it is already > > there. So I could not understand or make sense of the patch Conor sent > > in reply to my V2. > > That is just how it works. You need to spell out exactly which > combinations are permitted. The current entry for s4 says that s4 is > only permitted in isolation. > Since you are adding "amlogic,meson-t7-uart", "amlogic,meson-s4-uart" > you need to explicitly allow that combination. You'll notice if you look > at the file that the gx uart appears more than once. > > Given the g12a was the most recently added compatible, it might make > sense to follow the pattern that it had set, given the thing your > original patch copied the match data from was the g12a. That change to > the dt-binding would look like: > diff --git a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml > index 01ec45b3b406..eae11e87b88a 100644 > --- a/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml > +++ b/Documentation/devicetree/bindings/serial/amlogic,meson-uart.yaml > @@ -50,6 +50,13 @@ properties: > items: > - const: amlogic,meson-g12a-uart > - const: amlogic,meson-gx-uart > + - description: > + Everything-Else power domain UART controller on G12A compatible SoCs > + items: > + - enum: > + - amlogic,meson-t7-uart > + - const: amlogic,meson-g12a-uart > + - const: amlogic,meson-gx-uart > > reg: > maxItems: 1 > > /I/ don't really care whether you do that, or do the s4 version of it, > but following the most recent pattern might make more sense. When I > suggested s4, it was because I only looked at the driver patch rather > than the code itself. > > > Krzysztof, I will check again with dtbs_check and re-send. > > Cheers, > Conor. I am struggling to understand this. Everything I try fails the check.
