Re: [PATCH v1 1/2] dt-bindings: display: panel: raspberrypi: Add compatible property for waveshare 7inch touchscreen panel
On Thu, Dec 07, 2023 at 11:48:56AM +0800, Shengyang Chen wrote: > Hi, Conor > > thanks for comment > > On 2023/12/6 23:40, Conor Dooley wrote: > > On Wed, Dec 06, 2023 at 05:43:48PM +0800, Shengyang Chen wrote: > >> Hi, Conor > >> > >> On 2023/11/24 20:31, Conor Dooley wrote: > >> > On Fri, Nov 24, 2023 at 06:44:50PM +0800, Shengyang Chen wrote: > >> >> The waveshare 7inch touchscreen panel is a kind of raspberrypi pi > >> >> panel > >> > > >> > Can you be more specific about what "is a kind of rpi panel" means? > >> > Are they using identical chips as controllers or something like that? > >> > > >> > >> Wareshare panel has same i2c slave address and registers address with > >> the original raspberry pi panel. They both use Atmel firmware and they > >> got same reg id. It can be operated by using the driver of raspberry pi > >> driver > >> after some change of the code. So I suppose it may be a kind of raspberry > >> pi panel > >> and discribe it in this way. It's my own judgement. Sorry about that. > >> Maybe just like Dave said, It cloned the behaviour of the raspberri pi > >> panel. > >> I will change the discribtion in next version to not make other confused. > >> > >> By the way, we will try Stefan's method before next version. > >> The method we used in this patch may be abandoned if Stefan's method is > >> verified in our platform. > >> At that time yaml may also be changed to fit new method. > > > > I don't know what Stefan's approach is, but I do not think that a > > bindings patch should be dropped. The waveshare might be a clone, but it > > is a distinct device. If the same driver can control both, then the > > compatible setups that should be permitted are: > > compatible = "raspberrypi,7inch-touchscreen-panel"; > > and > > compatible = "waveshare,7inch-touchscreen-panel", > > "raspberrypi,7inch-touchscreen-panel"; > If we use Stenfan's method, we can reuse the code of panel-simple.c > we may submit our patch to > /Documentation/devicetree/bindings/display/panel/panel-simple.yaml > /drivers/gpu/drm/panel/panel-simple.c > as a new panel porting. That may less confuse. As long as you provide a specific compatible, and not re-use the rpi one, that's fine. It just sounded like you were intending to reuse that here, but from this message it seems like I misunderstood. Thanks, Conor. signature.asc Description: PGP signature
Re: [PATCH v1 1/2] dt-bindings: display: panel: raspberrypi: Add compatible property for waveshare 7inch touchscreen panel
Hi, Conor
thanks for comment
On 2023/12/6 23:40, Conor Dooley wrote:
> On Wed, Dec 06, 2023 at 05:43:48PM +0800, Shengyang Chen wrote:
>> Hi, Conor
>>
>> On 2023/11/24 20:31, Conor Dooley wrote:
>> > On Fri, Nov 24, 2023 at 06:44:50PM +0800, Shengyang Chen wrote:
>> >> The waveshare 7inch touchscreen panel is a kind of raspberrypi pi
>> >> panel
>> >
>> > Can you be more specific about what "is a kind of rpi panel" means?
>> > Are they using identical chips as controllers or something like that?
>> >
>>
>> Wareshare panel has same i2c slave address and registers address with
>> the original raspberry pi panel. They both use Atmel firmware and they
>> got same reg id. It can be operated by using the driver of raspberry pi
>> driver
>> after some change of the code. So I suppose it may be a kind of raspberry pi
>> panel
>> and discribe it in this way. It's my own judgement. Sorry about that.
>> Maybe just like Dave said, It cloned the behaviour of the raspberri pi panel.
>> I will change the discribtion in next version to not make other confused.
>>
>> By the way, we will try Stefan's method before next version.
>> The method we used in this patch may be abandoned if Stefan's method is
>> verified in our platform.
>> At that time yaml may also be changed to fit new method.
>
> I don't know what Stefan's approach is, but I do not think that a
> bindings patch should be dropped. The waveshare might be a clone, but it
> is a distinct device. If the same driver can control both, then the
> compatible setups that should be permitted are:
> compatible = "raspberrypi,7inch-touchscreen-panel";
> and
> compatible = "waveshare,7inch-touchscreen-panel",
> "raspberrypi,7inch-touchscreen-panel";
>
> Cheers,
> Conor.
>
Here is our consideration of this submit:
Although Waveshare panel reuse the driver of raspberry pi panel, they are
different in probing process
and panel parameters.
we try to use compatible and data to distinguish these two panel
Here are the reference
driver part:
https://elixir.bootlin.com/linux/v6.7-rc3/source/drivers/gpu/drm/panel/panel-simple.c
dt-binding part:
https://elixir.bootlin.com/linux/v6.7-rc3/source/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
For example:
in driver part:
in drivers/gpu/drm/panel/panel-simple.c:#in line 4189
--
we can got different compatible with its own data.
static const struct of_device_id platform_of_match[] = {
//the of_match array list
{
.compatible = "ampire,am-1280800n3tzqw-t00h",
.data = &ire_am_1280800n3tzqw_t00h, //we define our
panel parameter or special panel function, which can distinguish different
panels
}, {
.compatible = "ampire,am-480272h3tmqw-t01h",
.data = &ire_am_480272h3tmqw_t01h,
},
...
...
}
===
in drivers/gpu/drm/panel/panel-simple.c:#in line 4611
--
we can use the generic probing process to probe our driver
after getting its own data.
static int panel_simple_platform_probe(struct platform_device *pdev)
{
const struct panel_desc *desc;
desc = of_device_get_match_data(&pdev->dev);//we
get our panel parameter
if (!desc)
return -ENODEV;
return panel_simple_probe(&pdev->dev, desc);//probe
with returned data
}
in yamel part:
in /Documentation/devicetree/bindings/display/panel/panel-simple.yaml#in line 33
--
We refer to this approach, adding our compatible to the yaml of raspberry pi
panel
properties:
compatible:
enum:
# compatible must be listed in alphabetical order, ordered by compatible.
# The description in the comment is mandatory for each compatible.
# Ampire AM-1280800N3TZQW-T00H 10.1" WQVGA TFT LCD panel
- ampire,am-1280800n3tzqw-t00h
# Ampire AM-480272H3TMQW-T01H 4.3" WQVGA TFT LCD panel
- ampire,am-480272h3tmqw-t01h
If we use Stenfan's method, we can reuse the code of panel-simple.c
we may submit our patch to
/Documentation/devicetree/bindings/display/panel/panel-simple.yaml
/drivers/gpu/drm/panel/panel-simple.c
as a new panel porting. That may less confuse.
here is Stenfan's method:
[1] - https://github.com/lategoodbye/rpi-zero/commits/v6.7-7inch-ts
[2] -
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/arch/arm64/boot/dts/freescale/imx8mm-venice-gw72xx-0x-rpidsi.dtso?h=v6.6.2&id=6b4da135
Re: [PATCH v1 1/2] dt-bindings: display: panel: raspberrypi: Add compatible property for waveshare 7inch touchscreen panel
On Wed, Dec 06, 2023 at 05:43:48PM +0800, Shengyang Chen wrote: > Hi, Conor > > On 2023/11/24 20:31, Conor Dooley wrote: > > On Fri, Nov 24, 2023 at 06:44:50PM +0800, Shengyang Chen wrote: > >> The waveshare 7inch touchscreen panel is a kind of raspberrypi pi > >> panel > > > > Can you be more specific about what "is a kind of rpi panel" means? > > Are they using identical chips as controllers or something like that? > > > > Wareshare panel has same i2c slave address and registers address with > the original raspberry pi panel. They both use Atmel firmware and they > got same reg id. It can be operated by using the driver of raspberry pi driver > after some change of the code. So I suppose it may be a kind of raspberry pi > panel > and discribe it in this way. It's my own judgement. Sorry about that. > Maybe just like Dave said, It cloned the behaviour of the raspberri pi panel. > I will change the discribtion in next version to not make other confused. > > By the way, we will try Stefan's method before next version. > The method we used in this patch may be abandoned if Stefan's method is > verified in our platform. > At that time yaml may also be changed to fit new method. I don't know what Stefan's approach is, but I do not think that a bindings patch should be dropped. The waveshare might be a clone, but it is a distinct device. If the same driver can control both, then the compatible setups that should be permitted are: compatible = "raspberrypi,7inch-touchscreen-panel"; and compatible = "waveshare,7inch-touchscreen-panel", "raspberrypi,7inch-touchscreen-panel"; Cheers, Conor. > >> and it can be drived by panel-raspberrypi-touchscreen.c. > >> Add compatible property for it. > >> > >> Signed-off-by: Keith Zhao > >> Signed-off-by: Shengyang Chen > >> --- > >> .../bindings/display/panel/raspberrypi,7inch-touchscreen.yaml | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >> diff --git > >> a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml > >> > >> b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml > >> index 22a083f7bc8e..e4e6cb4d4e5b 100644 > >> --- > >> a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml > >> +++ > >> b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml > >> @@ -22,7 +22,9 @@ description: |+ > >> > >> properties: > >>compatible: > >> -const: raspberrypi,7inch-touchscreen-panel > >> +enum: > >> + - raspberrypi,7inch-touchscreen-panel > >> + - waveshare,7inch-touchscreen-panel > >> > >>reg: > >> const: 0x45 > >> -- > >> 2.17.1 > >> > > > thanks. > > Best Regards, > Shengyang > signature.asc Description: PGP signature
Re: [PATCH v1 1/2] dt-bindings: display: panel: raspberrypi: Add compatible property for waveshare 7inch touchscreen panel
Hi, Conor On 2023/11/24 20:31, Conor Dooley wrote: > On Fri, Nov 24, 2023 at 06:44:50PM +0800, Shengyang Chen wrote: >> The waveshare 7inch touchscreen panel is a kind of raspberrypi pi >> panel > > Can you be more specific about what "is a kind of rpi panel" means? > Are they using identical chips as controllers or something like that? > Wareshare panel has same i2c slave address and registers address with the original raspberry pi panel. They both use Atmel firmware and they got same reg id. It can be operated by using the driver of raspberry pi driver after some change of the code. So I suppose it may be a kind of raspberry pi panel and discribe it in this way. It's my own judgement. Sorry about that. Maybe just like Dave said, It cloned the behaviour of the raspberri pi panel. I will change the discribtion in next version to not make other confused. By the way, we will try Stefan's method before next version. The method we used in this patch may be abandoned if Stefan's method is verified in our platform. At that time yaml may also be changed to fit new method. >> and it can be drived by panel-raspberrypi-touchscreen.c. >> Add compatible property for it. >> >> Signed-off-by: Keith Zhao >> Signed-off-by: Shengyang Chen >> --- >> .../bindings/display/panel/raspberrypi,7inch-touchscreen.yaml | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git >> a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml >> >> b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml >> index 22a083f7bc8e..e4e6cb4d4e5b 100644 >> --- >> a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml >> +++ >> b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml >> @@ -22,7 +22,9 @@ description: |+ >> >> properties: >>compatible: >> -const: raspberrypi,7inch-touchscreen-panel >> +enum: >> + - raspberrypi,7inch-touchscreen-panel >> + - waveshare,7inch-touchscreen-panel >> >>reg: >> const: 0x45 >> -- >> 2.17.1 >> thanks. Best Regards, Shengyang
[PATCH v1 1/2] dt-bindings: display: panel: raspberrypi: Add compatible property for waveshare 7inch touchscreen panel
The waveshare 7inch touchscreen panel is a kind of raspberrypi pi panel and it can be drived by panel-raspberrypi-touchscreen.c. Add compatible property for it. Signed-off-by: Keith Zhao Signed-off-by: Shengyang Chen --- .../bindings/display/panel/raspberrypi,7inch-touchscreen.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml index 22a083f7bc8e..e4e6cb4d4e5b 100644 --- a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml +++ b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml @@ -22,7 +22,9 @@ description: |+ properties: compatible: -const: raspberrypi,7inch-touchscreen-panel +enum: + - raspberrypi,7inch-touchscreen-panel + - waveshare,7inch-touchscreen-panel reg: const: 0x45 -- 2.17.1
Re: [PATCH v1 1/2] dt-bindings: display: panel: raspberrypi: Add compatible property for waveshare 7inch touchscreen panel
On Fri, Nov 24, 2023 at 06:44:50PM +0800, Shengyang Chen wrote: > The waveshare 7inch touchscreen panel is a kind of raspberrypi pi > panel Can you be more specific about what "is a kind of rpi panel" means? Are they using identical chips as controllers or something like that? > and it can be drived by panel-raspberrypi-touchscreen.c. > Add compatible property for it. > > Signed-off-by: Keith Zhao > Signed-off-by: Shengyang Chen > --- > .../bindings/display/panel/raspberrypi,7inch-touchscreen.yaml | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git > a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml > > b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml > index 22a083f7bc8e..e4e6cb4d4e5b 100644 > --- > a/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml > +++ > b/Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.yaml > @@ -22,7 +22,9 @@ description: |+ > > properties: >compatible: > -const: raspberrypi,7inch-touchscreen-panel > +enum: > + - raspberrypi,7inch-touchscreen-panel > + - waveshare,7inch-touchscreen-panel > >reg: > const: 0x45 > -- > 2.17.1 > signature.asc Description: PGP signature
