On 25/10/2024 18:19, Doug Anderson wrote: > Hi, > > On Fri, Oct 25, 2024 at 8:59 AM Rob Herring <robh@xxxxxxxxxx> wrote: >> >> On Fri, Oct 25, 2024 at 10:29 AM Doug Anderson <dianders@xxxxxxxxxxxx> wrote: >>> >>> Charles, >>> >>> On Fri, Oct 25, 2024 at 5:03 AM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: >>>> >>>>> +properties: >>>>> + compatible: >>>>> + enum: >>>>> + - goodix,gt7986u-spi >>>> >>>> Compatible is already documented and nothing here explains why we should >>>> spi variant. >>>> >>>>> + >>>>> + reg: >>>>> + maxItems: 1 >>>>> + >>>>> + interrupts: >>>>> + maxItems: 1 >>>>> + >>>>> + reset-gpios: >>>>> + maxItems: 1 >>>>> + >>>>> + goodix,hid-report-addr: >>>> >>>> I do not see this patch addressing previous review. Sending something >>>> like this as v1 after long discussions also does not help. >>> >>> Krzysztof is right that it's better to wait until we get consensus on >>> the previous discussion before sending a new patch. I know you were >>> just trying to help move things forward, but because of the way the >>> email workflow works, sending a new version tends to fork the >>> discussion into two threads and adds confusion. >>> >>> I know Krzysztof and Rob have been silent during our recent >>> discussion, but it's also a long discussion. I've been assuming that >>> they will take some time to digest and reply in a little bit. If they >>> didn't, IMO it would have been reasonable to explicitly ask them for >>> feedback in the other thread after giving a bit of time. >> >> If the firmware creates fundamentally different interfaces, then >> different compatibles makes sense. If the same driver handles both bus >> interfaces, then 1 compatible should be fine. The addition of '-spi' >> to the compatible doesn't give any indication of a different >> programming model. I wouldn't care except for folks who will see it >> and just copy it when their only difference is the bus interface and >> we get to have the same discussion all over again. So if appending >> '-spi' is the only thing you can come up with, make it abundantly >> clear so that others don't blindly copy it. The commit msg is useful >> for convincing us, but not for that purpose. > > OK, makes sense. Charles: Can you think of any better description for > this interface than "goodix,gt7986u-spi"? I suppose you could make it > super obvious that it's running different firmware with > "goodix,gt7986u-spifw" and maybe that would be a little better. > > I think what Rob is asking for to make it super obvious is that in the > "description" of the binding you mention that in this case we're > running a substantially different firmware than GT7986U touchscreens > represented by the "goodix,gt7986u" binding and thus is considered a > distinct device. > > At this point, IMO you could wait until Monday in case Krzysztof wants > to add his $0.02 worth and then you could send a "v2" patch addressing > the comments so far, though of course you could continue to reply to > this thread if you have further questions / comments. And to re-iterate: both commit msg and hardware description in the binding must clearly explain this why another compatible was chosen for the same device. Best regards, Krzysztof
