[V2,1/4] dt-bindings: display: panel: Update NewVision NV3051D compatibles

From: Chris Morgan <macromorgan@hotmail.com>

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.

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". Now that I am adding the 3rd panel type of
"powkiddy,rk2023-panel" I am correcting the driver to do it the right
way by checking for the specific compatibles.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 .../devicetree/bindings/display/panel/newvision,nv3051d.yaml    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On 09/11/2023 22:50, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> 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.
> 
> 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". Now that I am adding the 3rd panel type of
> "powkiddy,rk2023-panel" I am correcting the driver to do it the right
> way by checking for the specific compatibles.

I don't understand how any of this driver behavior is a reason to drop
rg353v. You wrote two paragraphs to justify this removal, but I feel the
only reason is that rg353v is just not needed, because it is duplicating
rg353p? Is this right? You actually did not write it explicitly...

Best regards,
Krzysztof

On Fri, Nov 10, 2023 at 02:11:58PM +0100, Krzysztof Kozlowski wrote:
> On 09/11/2023 22:50, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > 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.
> > 
> > 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". Now that I am adding the 3rd panel type of
> > "powkiddy,rk2023-panel" I am correcting the driver to do it the right
> > way by checking for the specific compatibles.
> 
> I don't understand how any of this driver behavior is a reason to drop
> rg353v. You wrote two paragraphs to justify this removal, but I feel the
> only reason is that rg353v is just not needed, because it is duplicating
> rg353p? Is this right? You actually did not write it explicitly...

Sorry if I wasn't clear, I did note that the rg353p-panel is exactly
identical to the rg353v-panel. Should I add additional details beyond
that to clarify?

Thank you.

> 
> Best regards,
> Krzysztof
>

On 10/11/2023 15:28, Chris Morgan wrote:
> On Fri, Nov 10, 2023 at 02:11:58PM +0100, Krzysztof Kozlowski wrote:
>> On 09/11/2023 22:50, Chris Morgan wrote:
>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>
>>> 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.
>>>
>>> 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". Now that I am adding the 3rd panel type of
>>> "powkiddy,rk2023-panel" I am correcting the driver to do it the right
>>> way by checking for the specific compatibles.
>>
>> I don't understand how any of this driver behavior is a reason to drop
>> rg353v. You wrote two paragraphs to justify this removal, but I feel the
>> only reason is that rg353v is just not needed, because it is duplicating
>> rg353p? Is this right? You actually did not write it explicitly...
> 
> Sorry if I wasn't clear, I did note that the rg353p-panel is exactly
> identical to the rg353v-panel. Should I add additional details beyond
> that to clarify?

The entire paragraph about driver feels redundant. Your first paragraph
should still say why you remove it.

Best regards,
Krzysztof