On Tue, Oct 24, 2023 at 01:27:55PM -0500, Rob Herring wrote: > On Thu, Oct 19, 2023 at 09:50:38AM -0500, Chris Morgan wrote: > > On Thu, Oct 19, 2023 at 11:22:19AM +0200, Krzysztof Kozlowski wrote: > > > On 18/10/2023 18:18, Chris Morgan wrote: > > > > From: Chris Morgan <macromorgan@xxxxxxxxxxx> > > > > > > > > Update the NewVision NV3051D compatible strings by adding a new panel, > > > > the powkiddy,rk2023-panel, and removing another entry, the > > > > anbernic,rg353v-panel. The rg353v-panel is exactly identical to the > > > > rg353p-panel and is not currently in use by any existing device tree. > > > > The rk2023-panel is similar to the rg353p-panel but has slightly > > > > different timings. > > > > > > This still does not explain me why do you want to remove old panel. > > > > When I originally wrote the driver I only had one piece of hardware > > and I set the compatible string in the driver as newvision,nv3051d. > > Unfortunately since then I've found 2 more devices in use that are > > *just* different enough to require the driver to do things a bit > > differently. In the case of the anbernic,rg351v-panel I need to > > enable a new DSI flag; in the case of the powkiddy,rk2023-panel I need > > to decrease the vertical back porch and drop the higher frequency > > timings. > > > > The best way to accomplish this was to change the strategy from having > > a single binding in the driver of newvision,nv3051d to a binding for > > each distinct hardware where the differences apply. > > Exactly why the DT maintainers annoyingly ask for specific compatible > strings which may not be used immediately. You're not wrong. Sorry for making this difficult. I should have done it this way from the start. > > > Note that I've > > looked at querying the DSI panel ID, but for each device the value > > is identical (so it can't be used to differentiate the hardware sadly). > > So the driver now has 3 different compatible strings. I could in this > > case add a 4th compatible string of anbernic,rg353v-panel but it would > > be identical to anbernic,rg353p-panel. For the moment we are using > > anbernic,rg353p-panel everywhere (including the rg353v), so it makes > > sense to drop this unused value while we can, at least to me. > > Your reasoning is the compatible string is unused, so remove it. > > If there's some reasoning about how the 2 panels are the same hardware > or the rg353v is never going to be used or show up at some point, then > that would be a reason to remove. The compatible string of 353v-panel is unused, and the hardware is identical to the 353p-panel (so only one string is necessary). Sorry if that wasn't clear. Panel 1 - The original anbernic,rg353p-panel which is also anbernic,rg353v-panel. Panel 2 - anbernic,rg351v-panel. This is almost identical to Panel 1 except it requires an additional flag. Panel 3 - powkiddy,rk2023-panel. This is almost identical to Panel 1 except it requires a change to the VBP timing parameter and isn't tolerant of speeds much higher than 60hz. The issue I had is I originally wrote the driver checking for the newvision,nv3051d compatible string which worked fine when there was only 1 panel type. When I added support for the 351v-panel I *should* have changed how the compatible string was handled, but instead I simply added a check in the probe function to look for the secondary string of "anbernic,rg351v-panel". When the 3rd panel type of "powkiddy,rk2023-panel" was needed I took this time to correct the driver and do it the right way by checking for the specific compatibles. Thank you, and sorry for the headaches this caused you. Chris > > You could also say the rg353v is just wrong because it should have a > fallback compatible to rg353p and rather than fix it, just remove it > for now since there are no known users of it. > > Rob
