| Message ID | 20231109215007.66826-2-macroalpha82@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | rockchip: Add Powkiddy RK2023 | expand |
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
diff --git a/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml index cce775a87f87..7a634fbc465e 100644 --- a/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml +++ b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml @@ -21,7 +21,7 @@ properties: - enum: - anbernic,rg351v-panel - anbernic,rg353p-panel - - anbernic,rg353v-panel + - powkiddy,rk2023-panel - const: newvision,nv3051d reg: true