2023-09-22 09:29:14

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <[email protected]> wrote:
> > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:

> > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > user to select.
> >
> > That seems like a bad idea, we want to be able to build one kernel that
> > works for all hardware at the same time.
> >
> I see, so I should take that back?
> I'll explain more about it.

Are there particular ICs where the firmware would always be in flash and
others where it would never be? Or is this a choice made by the board or
system designer?

Thanks,
Conor.


Attachments:
(No filename) (764.00 B)
signature.asc (235.00 B)
Download all attachments

2023-09-22 18:33:02

by yang tylor

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <[email protected]> wrote:
>
> On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <[email protected]> wrote:
> > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
>
> > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > > user to select.
> > >
> > > That seems like a bad idea, we want to be able to build one kernel that
> > > works for all hardware at the same time.
> > >
> > I see, so I should take that back?
> > I'll explain more about it.
>
> Are there particular ICs where the firmware would always be in flash and
> others where it would never be? Or is this a choice made by the board or
> system designer?
>
Most cases it's about the system designer's decision. But some ICs may be forced
to use flash because of its architecture(multiple IC inside, need to
load firmware to
multiple IC's sram by master IC). But if there is no limitation on
this part, most system
designers will prefer flashless.

> Thanks,
> Conor.

Thanks,
Tylor

2023-09-22 18:57:34

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote:
> On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <[email protected]> wrote:
> >
> > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <[email protected]> wrote:
> > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
> >
> > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > > > user to select.
> > > >
> > > > That seems like a bad idea, we want to be able to build one kernel that
> > > > works for all hardware at the same time.
> > > >
> > > I see, so I should take that back?
> > > I'll explain more about it.
> >
> > Are there particular ICs where the firmware would always be in flash and
> > others where it would never be? Or is this a choice made by the board or
> > system designer?
> >
> Most cases it's about the system designer's decision. But some ICs may be forced
> to use flash because of its architecture(multiple IC inside, need to
> load firmware to
> multiple IC's sram by master IC). But if there is no limitation on
> this part, most system
> designers will prefer flashless.

Forgive me if I am not understanding correctly, there are some ICs that
will need to load the firmware from flash and there are some where it
will be a decision made by the designer of the board. Is the flash part
of the IC or is it an external flash chip?

Cheers,
Conor.


Attachments:
(No filename) (1.51 kB)
signature.asc (235.00 B)
Download all attachments

2023-09-25 06:48:05

by yang tylor

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <[email protected]> wrote:
>
> On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote:
> > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <[email protected]> wrote:
> > >
> > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <[email protected]> wrote:
> > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
> > >
> > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > > > > user to select.
> > > > >
> > > > > That seems like a bad idea, we want to be able to build one kernel that
> > > > > works for all hardware at the same time.
> > > > >
> > > > I see, so I should take that back?
> > > > I'll explain more about it.
> > >
> > > Are there particular ICs where the firmware would always be in flash and
> > > others where it would never be? Or is this a choice made by the board or
> > > system designer?
> > >
> > Most cases it's about the system designer's decision. But some ICs may be forced
> > to use flash because of its architecture(multiple IC inside, need to
> > load firmware to
> > multiple IC's sram by master IC). But if there is no limitation on
> > this part, most system
> > designers will prefer flashless.
>
> Forgive me if I am not understanding correctly, there are some ICs that
> will need to load the firmware from flash and there are some where it
> will be a decision made by the designer of the board. Is the flash part
> of the IC or is it an external flash chip?
>

Both are possible, it depends on the IC type. For TDDI, the IC is long
and thin, placed on panel PCB, flash will be located at the external
flash chip. For the OLED TP, IC is usually placed at FPC and its flash
is embedded, thus the IC size is large compared to TDDI. But from the
driver's perspective either external flash or embedded flash, the IC
itself will load firmware from flash automatically when reset pin is
released. Only if firmware is loading from the host storage system,
the driver needs to operate the IC in detail.

> Cheers,
> Conor.


Thanks,
Tylor

2023-09-25 11:37:37

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote:
> On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <[email protected]> wrote:
> >
> > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote:
> > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <[email protected]> wrote:
> > > >
> > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <[email protected]> wrote:
> > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
> > > >
> > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > > > > > user to select.
> > > > > >
> > > > > > That seems like a bad idea, we want to be able to build one kernel that
> > > > > > works for all hardware at the same time.
> > > > > >
> > > > > I see, so I should take that back?
> > > > > I'll explain more about it.
> > > >
> > > > Are there particular ICs where the firmware would always be in flash and
> > > > others where it would never be? Or is this a choice made by the board or
> > > > system designer?
> > > >
> > > Most cases it's about the system designer's decision. But some ICs may be forced
> > > to use flash because of its architecture(multiple IC inside, need to
> > > load firmware to
> > > multiple IC's sram by master IC). But if there is no limitation on
> > > this part, most system
> > > designers will prefer flashless.
> >
> > Forgive me if I am not understanding correctly, there are some ICs that
> > will need to load the firmware from flash and there are some where it
> > will be a decision made by the designer of the board. Is the flash part
> > of the IC or is it an external flash chip?
> >
>
> Both are possible, it depends on the IC type. For TDDI, the IC is long
> and thin, placed on panel PCB, flash will be located at the external
> flash chip. For the OLED TP, IC is usually placed at FPC and its flash
> is embedded, thus the IC size is large compared to TDDI. But from the
> driver's perspective either external flash or embedded flash, the IC
> itself will load firmware from flash automatically when reset pin is
> released. Only if firmware is loading from the host storage system,
> the driver needs to operate the IC in detail.


Since there are ICs that can use the external flash or have it loaded
from the host, it sounds like you do need a property to differentiate
between those cases.
Is it sufficient to just set the firmware-name property for these cases?
If the property exists, then you know you need to load firmware & what
its name is. If it doesn't, then the firmware either isn't needed or
will be automatically loaded from the external flash.


Attachments:
(No filename) (2.74 kB)
signature.asc (235.00 B)
Download all attachments

2023-09-25 15:33:15

by yang tylor

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <[email protected]> wrote:
>
> On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote:
> > On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <[email protected]> wrote:
> > >
> > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote:
> > > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <[email protected]> wrote:
> > > > >
> > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> > > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <[email protected]> wrote:
> > > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
> > > > >
> > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > > > > > > user to select.
> > > > > > >
> > > > > > > That seems like a bad idea, we want to be able to build one kernel that
> > > > > > > works for all hardware at the same time.
> > > > > > >
> > > > > > I see, so I should take that back?
> > > > > > I'll explain more about it.
> > > > >
> > > > > Are there particular ICs where the firmware would always be in flash and
> > > > > others where it would never be? Or is this a choice made by the board or
> > > > > system designer?
> > > > >
> > > > Most cases it's about the system designer's decision. But some ICs may be forced
> > > > to use flash because of its architecture(multiple IC inside, need to
> > > > load firmware to
> > > > multiple IC's sram by master IC). But if there is no limitation on
> > > > this part, most system
> > > > designers will prefer flashless.
> > >
> > > Forgive me if I am not understanding correctly, there are some ICs that
> > > will need to load the firmware from flash and there are some where it
> > > will be a decision made by the designer of the board. Is the flash part
> > > of the IC or is it an external flash chip?
> > >
> >
> > Both are possible, it depends on the IC type. For TDDI, the IC is long
> > and thin, placed on panel PCB, flash will be located at the external
> > flash chip. For the OLED TP, IC is usually placed at FPC and its flash
> > is embedded, thus the IC size is large compared to TDDI. But from the
> > driver's perspective either external flash or embedded flash, the IC
> > itself will load firmware from flash automatically when reset pin is
> > released. Only if firmware is loading from the host storage system,
> > the driver needs to operate the IC in detail.
>
>
> Since there are ICs that can use the external flash or have it loaded
> from the host, it sounds like you do need a property to differentiate
> between those cases.
Yep.

> Is it sufficient to just set the firmware-name property for these cases?
> If the property exists, then you know you need to load firmware & what
> its name is. If it doesn't, then the firmware either isn't needed or
> will be automatically loaded from the external flash.
We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.
So we'll look for it when no-flash-flag is specified. In our experience,
forcing a prefix firmware name helps the user to aware what firmware
they are dealing with.

Thanks,
Tylor

2023-09-26 09:07:33

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:
> On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <[email protected]> wrote:
> >
> > On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote:
> > > On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <[email protected]> wrote:
> > > >
> > > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote:
> > > > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <[email protected]> wrote:
> > > > > >
> > > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> > > > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <[email protected]> wrote:
> > > > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
> > > > > >
> > > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > > > > > > > user to select.
> > > > > > > >
> > > > > > > > That seems like a bad idea, we want to be able to build one kernel that
> > > > > > > > works for all hardware at the same time.
> > > > > > > >
> > > > > > > I see, so I should take that back?
> > > > > > > I'll explain more about it.
> > > > > >
> > > > > > Are there particular ICs where the firmware would always be in flash and
> > > > > > others where it would never be? Or is this a choice made by the board or
> > > > > > system designer?
> > > > > >
> > > > > Most cases it's about the system designer's decision. But some ICs may be forced
> > > > > to use flash because of its architecture(multiple IC inside, need to
> > > > > load firmware to
> > > > > multiple IC's sram by master IC). But if there is no limitation on
> > > > > this part, most system
> > > > > designers will prefer flashless.
> > > >
> > > > Forgive me if I am not understanding correctly, there are some ICs that
> > > > will need to load the firmware from flash and there are some where it
> > > > will be a decision made by the designer of the board. Is the flash part
> > > > of the IC or is it an external flash chip?
> > > >
> > >
> > > Both are possible, it depends on the IC type. For TDDI, the IC is long
> > > and thin, placed on panel PCB, flash will be located at the external
> > > flash chip. For the OLED TP, IC is usually placed at FPC and its flash
> > > is embedded, thus the IC size is large compared to TDDI. But from the
> > > driver's perspective either external flash or embedded flash, the IC
> > > itself will load firmware from flash automatically when reset pin is
> > > released. Only if firmware is loading from the host storage system,
> > > the driver needs to operate the IC in detail.
> >
> >
> > Since there are ICs that can use the external flash or have it loaded
> > from the host, it sounds like you do need a property to differentiate
> > between those cases.
> Yep.
>
> > Is it sufficient to just set the firmware-name property for these cases?
> > If the property exists, then you know you need to load firmware & what
> > its name is. If it doesn't, then the firmware either isn't needed or
> > will be automatically loaded from the external flash.

> We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.

How do you intend generating the name of the firmware file? I assume the
same firmware doesn't work on every IC, so you'll need to pick a
different one depending on the compatible?

> So we'll look for it when no-flash-flag is specified. In our experience,
> forcing a prefix firmware name helps the user to aware what firmware
> they are dealing with.


Attachments:
(No filename) (3.54 kB)
signature.asc (235.00 B)
Download all attachments

2023-09-26 12:26:43

by yang tylor

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <[email protected]> wrote:
>
> On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:
> > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <[email protected]> wrote:
> > >
> > > On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote:
> > > > On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <[email protected]> wrote:
> > > > >
> > > > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote:
> > > > > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <[email protected]> wrote:
> > > > > > >
> > > > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> > > > > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <[email protected]> wrote:
> > > > > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
> > > > > > >
> > > > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > > > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > > > > > > > > user to select.
> > > > > > > > >
> > > > > > > > > That seems like a bad idea, we want to be able to build one kernel that
> > > > > > > > > works for all hardware at the same time.
> > > > > > > > >
> > > > > > > > I see, so I should take that back?
> > > > > > > > I'll explain more about it.
> > > > > > >
> > > > > > > Are there particular ICs where the firmware would always be in flash and
> > > > > > > others where it would never be? Or is this a choice made by the board or
> > > > > > > system designer?
> > > > > > >
> > > > > > Most cases it's about the system designer's decision. But some ICs may be forced
> > > > > > to use flash because of its architecture(multiple IC inside, need to
> > > > > > load firmware to
> > > > > > multiple IC's sram by master IC). But if there is no limitation on
> > > > > > this part, most system
> > > > > > designers will prefer flashless.
> > > > >
> > > > > Forgive me if I am not understanding correctly, there are some ICs that
> > > > > will need to load the firmware from flash and there are some where it
> > > > > will be a decision made by the designer of the board. Is the flash part
> > > > > of the IC or is it an external flash chip?
> > > > >
> > > >
> > > > Both are possible, it depends on the IC type. For TDDI, the IC is long
> > > > and thin, placed on panel PCB, flash will be located at the external
> > > > flash chip. For the OLED TP, IC is usually placed at FPC and its flash
> > > > is embedded, thus the IC size is large compared to TDDI. But from the
> > > > driver's perspective either external flash or embedded flash, the IC
> > > > itself will load firmware from flash automatically when reset pin is
> > > > released. Only if firmware is loading from the host storage system,
> > > > the driver needs to operate the IC in detail.
> > >
> > >
> > > Since there are ICs that can use the external flash or have it loaded
> > > from the host, it sounds like you do need a property to differentiate
> > > between those cases.
> > Yep.
> >
> > > Is it sufficient to just set the firmware-name property for these cases?
> > > If the property exists, then you know you need to load firmware & what
> > > its name is. If it doesn't, then the firmware either isn't needed or
> > > will be automatically loaded from the external flash.
>
> > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.
>
> How do you intend generating the name of the firmware file? I assume the
> same firmware doesn't work on every IC, so you'll need to pick a
> different one depending on the compatible?
>
If considering a firmware library line-up for all the incoming panels
of this driver.
We would use PID as part of the file name. Because all the support panels would
have a unique PID associated. Which will make the firmware name like
himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load
at no flash condition. Thus PID information is required in dts when
no-flash-flag
is specified.

> > So we'll look for it when no-flash-flag is specified. In our experience,
> > forcing a prefix firmware name helps the user to aware what firmware
> > they are dealing with.

If a more simple solution for no-flash condition is needed, as you mentioned,
specifying a firmware name in dts would be the best. Otherwise, a
no-flash-flag and
PID information needs to be added in dts.

Thanks,
Tylor

2023-09-26 13:02:25

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote:
> On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <[email protected]> wrote:
> >
> > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:
> > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <[email protected]> wrote:
> > > >
> > > > On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote:
> > > > > On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <[email protected]> wrote:
> > > > > >
> > > > > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote:
> > > > > > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <[email protected]> wrote:
> > > > > > > >
> > > > > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> > > > > > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <[email protected]> wrote:
> > > > > > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
> > > > > > > >
> > > > > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > > > > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > > > > > > > > > user to select.
> > > > > > > > > >
> > > > > > > > > > That seems like a bad idea, we want to be able to build one kernel that
> > > > > > > > > > works for all hardware at the same time.
> > > > > > > > > >
> > > > > > > > > I see, so I should take that back?
> > > > > > > > > I'll explain more about it.
> > > > > > > >
> > > > > > > > Are there particular ICs where the firmware would always be in flash and
> > > > > > > > others where it would never be? Or is this a choice made by the board or
> > > > > > > > system designer?
> > > > > > > >
> > > > > > > Most cases it's about the system designer's decision. But some ICs may be forced
> > > > > > > to use flash because of its architecture(multiple IC inside, need to
> > > > > > > load firmware to
> > > > > > > multiple IC's sram by master IC). But if there is no limitation on
> > > > > > > this part, most system
> > > > > > > designers will prefer flashless.
> > > > > >
> > > > > > Forgive me if I am not understanding correctly, there are some ICs that
> > > > > > will need to load the firmware from flash and there are some where it
> > > > > > will be a decision made by the designer of the board. Is the flash part
> > > > > > of the IC or is it an external flash chip?
> > > > > >
> > > > >
> > > > > Both are possible, it depends on the IC type. For TDDI, the IC is long
> > > > > and thin, placed on panel PCB, flash will be located at the external
> > > > > flash chip. For the OLED TP, IC is usually placed at FPC and its flash
> > > > > is embedded, thus the IC size is large compared to TDDI. But from the
> > > > > driver's perspective either external flash or embedded flash, the IC
> > > > > itself will load firmware from flash automatically when reset pin is
> > > > > released. Only if firmware is loading from the host storage system,
> > > > > the driver needs to operate the IC in detail.
> > > >
> > > >
> > > > Since there are ICs that can use the external flash or have it loaded
> > > > from the host, it sounds like you do need a property to differentiate
> > > > between those cases.
> > > Yep.
> > >
> > > > Is it sufficient to just set the firmware-name property for these cases?
> > > > If the property exists, then you know you need to load firmware & what
> > > > its name is. If it doesn't, then the firmware either isn't needed or
> > > > will be automatically loaded from the external flash.
> >
> > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.
> >
> > How do you intend generating the name of the firmware file? I assume the
> > same firmware doesn't work on every IC, so you'll need to pick a
> > different one depending on the compatible?
> >
> If considering a firmware library line-up for all the incoming panels
> of this driver.
> We would use PID as part of the file name. Because all the support panels would
> have a unique PID associated. Which will make the firmware name like
> himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load
> at no flash condition. Thus PID information is required in dts when
> no-flash-flag
> is specified.

Firstly, where does the "xxx" come from?
And you're making it sound more like having firmware-name is suitable
for this use case, given you need to determine the name of the file to
use based on something that is hardware specific but is not
dynamically detectable.

Thanks,
Conor.

> > > So we'll look for it when no-flash-flag is specified. In our experience,
> > > forcing a prefix firmware name helps the user to aware what firmware
> > > they are dealing with.
>
> If a more simple solution for no-flash condition is needed, as you mentioned,
> specifying a firmware name in dts would be the best. Otherwise, a
> no-flash-flag and
> PID information needs to be added in dts.



Attachments:
(No filename) (4.88 kB)
signature.asc (235.00 B)
Download all attachments

2023-09-28 04:32:26

by yang tylor

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <[email protected]> wrote:
>
> On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote:
> > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <[email protected]> wrote:
> > >
> > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:
> > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <[email protected]> wrote:
> > > > >
> > > > > On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote:
> > > > > > On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <[email protected]> wrote:
> > > > > > >
> > > > > > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote:
> > > > > > > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <[email protected]> wrote:
> > > > > > > > >
> > > > > > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> > > > > > > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <[email protected]> wrote:
> > > > > > > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
> > > > > > > > >
> > > > > > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > > > > > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > > > > > > > > > > user to select.
> > > > > > > > > > >
> > > > > > > > > > > That seems like a bad idea, we want to be able to build one kernel that
> > > > > > > > > > > works for all hardware at the same time.
> > > > > > > > > > >
> > > > > > > > > > I see, so I should take that back?
> > > > > > > > > > I'll explain more about it.
> > > > > > > > >
> > > > > > > > > Are there particular ICs where the firmware would always be in flash and
> > > > > > > > > others where it would never be? Or is this a choice made by the board or
> > > > > > > > > system designer?
> > > > > > > > >
> > > > > > > > Most cases it's about the system designer's decision. But some ICs may be forced
> > > > > > > > to use flash because of its architecture(multiple IC inside, need to
> > > > > > > > load firmware to
> > > > > > > > multiple IC's sram by master IC). But if there is no limitation on
> > > > > > > > this part, most system
> > > > > > > > designers will prefer flashless.
> > > > > > >
> > > > > > > Forgive me if I am not understanding correctly, there are some ICs that
> > > > > > > will need to load the firmware from flash and there are some where it
> > > > > > > will be a decision made by the designer of the board. Is the flash part
> > > > > > > of the IC or is it an external flash chip?
> > > > > > >
> > > > > >
> > > > > > Both are possible, it depends on the IC type. For TDDI, the IC is long
> > > > > > and thin, placed on panel PCB, flash will be located at the external
> > > > > > flash chip. For the OLED TP, IC is usually placed at FPC and its flash
> > > > > > is embedded, thus the IC size is large compared to TDDI. But from the
> > > > > > driver's perspective either external flash or embedded flash, the IC
> > > > > > itself will load firmware from flash automatically when reset pin is
> > > > > > released. Only if firmware is loading from the host storage system,
> > > > > > the driver needs to operate the IC in detail.
> > > > >
> > > > >
> > > > > Since there are ICs that can use the external flash or have it loaded
> > > > > from the host, it sounds like you do need a property to differentiate
> > > > > between those cases.
> > > > Yep.
> > > >
> > > > > Is it sufficient to just set the firmware-name property for these cases?
> > > > > If the property exists, then you know you need to load firmware & what
> > > > > its name is. If it doesn't, then the firmware either isn't needed or
> > > > > will be automatically loaded from the external flash.
> > >
> > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.
> > >
> > > How do you intend generating the name of the firmware file? I assume the
> > > same firmware doesn't work on every IC, so you'll need to pick a
> > > different one depending on the compatible?
> > >
> > If considering a firmware library line-up for all the incoming panels
> > of this driver.
> > We would use PID as part of the file name. Because all the support panels would
> > have a unique PID associated. Which will make the firmware name like
> > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load
> > at no flash condition. Thus PID information is required in dts when
> > no-flash-flag
> > is specified.
>
> Firstly, where does the "xxx" come from?
> And you're making it sound more like having firmware-name is suitable
> for this use case, given you need to determine the name of the file to
> use based on something that is hardware specific but is not
> dynamically detectable.
Current driver patch uses a prefix name "himax_i2chid" which comes
from the previous project
and seems not suitable for this condition, so I use "xxx" and plan to
replace it in the next version.
For finding firmware, I think both solutions are reasonable.
- provide firmware name directly: implies no-flash and use user
specified firmware, no PID info.
- provide no-flash-flag and PID info: loading firmware from organized
names with PID info.
I prefer the 2nd solution, but it needs more properties in dts. 1st
has less properties and more
intuitive.

I don't know which one is more acceptable by the community, as you
know I'm a newbie here.

Thanks,
Tylor

2023-09-28 17:57:33

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

On Thu, Sep 28, 2023 at 10:12:41AM +0800, yang tylor wrote:
> On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <[email protected]> wrote:
> > On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote:
> > > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <[email protected]> wrote:
> > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:
> > > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <[email protected]> wrote:
> > > > > > On Mon, Sep 25, 2023 at 09:44:21AM +0800, yang tylor wrote:
> > > > > > > On Fri, Sep 22, 2023 at 11:31 PM Conor Dooley <[email protected]> wrote:
> > > > > > > >
> > > > > > > > On Fri, Sep 22, 2023 at 05:43:54PM +0800, yang tylor wrote:
> > > > > > > > > On Fri, Sep 22, 2023 at 5:22 PM Conor Dooley <[email protected]> wrote:
> > > > > > > > > >
> > > > > > > > > > On Fri, Sep 22, 2023 at 03:56:25PM +0800, yang tylor wrote:
> > > > > > > > > > > On Tue, Sep 19, 2023 at 7:09 PM Conor Dooley <[email protected]> wrote:
> > > > > > > > > > > > On Tue, Sep 19, 2023 at 05:31:29PM +0800, yang tylor wrote:
> > > > > > > > > >
> > > > > > > > > > > > > The behavior of "himax,boot_time_fw_upgrade" seems not stable and
> > > > > > > > > > > > > should be removed. "himax,fw_in_flash", I use the kernel config for
> > > > > > > > > > > > > user to select.
> > > > > > > > > > > >
> > > > > > > > > > > > That seems like a bad idea, we want to be able to build one kernel that
> > > > > > > > > > > > works for all hardware at the same time.
> > > > > > > > > > > >
> > > > > > > > > > > I see, so I should take that back?
> > > > > > > > > > > I'll explain more about it.
> > > > > > > > > >
> > > > > > > > > > Are there particular ICs where the firmware would always be in flash and
> > > > > > > > > > others where it would never be? Or is this a choice made by the board or
> > > > > > > > > > system designer?
> > > > > > > > > >
> > > > > > > > > Most cases it's about the system designer's decision. But some ICs may be forced
> > > > > > > > > to use flash because of its architecture(multiple IC inside, need to
> > > > > > > > > load firmware to
> > > > > > > > > multiple IC's sram by master IC). But if there is no limitation on
> > > > > > > > > this part, most system
> > > > > > > > > designers will prefer flashless.
> > > > > > > >
> > > > > > > > Forgive me if I am not understanding correctly, there are some ICs that
> > > > > > > > will need to load the firmware from flash and there are some where it
> > > > > > > > will be a decision made by the designer of the board. Is the flash part
> > > > > > > > of the IC or is it an external flash chip?
> > > > > > > >
> > > > > > >
> > > > > > > Both are possible, it depends on the IC type. For TDDI, the IC is long
> > > > > > > and thin, placed on panel PCB, flash will be located at the external
> > > > > > > flash chip. For the OLED TP, IC is usually placed at FPC and its flash
> > > > > > > is embedded, thus the IC size is large compared to TDDI. But from the
> > > > > > > driver's perspective either external flash or embedded flash, the IC
> > > > > > > itself will load firmware from flash automatically when reset pin is
> > > > > > > released. Only if firmware is loading from the host storage system,
> > > > > > > the driver needs to operate the IC in detail.
> > > > > >
> > > > > >
> > > > > > Since there are ICs that can use the external flash or have it loaded
> > > > > > from the host, it sounds like you do need a property to differentiate
> > > > > > between those cases.
> > > > > Yep.
> > > > >
> > > > > > Is it sufficient to just set the firmware-name property for these cases?
> > > > > > If the property exists, then you know you need to load firmware & what
> > > > > > its name is. If it doesn't, then the firmware either isn't needed or
> > > > > > will be automatically loaded from the external flash.
> > > >
> > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.
> > > >
> > > > How do you intend generating the name of the firmware file? I assume the
> > > > same firmware doesn't work on every IC, so you'll need to pick a
> > > > different one depending on the compatible?
> > > >
> > > If considering a firmware library line-up for all the incoming panels
> > > of this driver.
> > > We would use PID as part of the file name. Because all the support panels would
> > > have a unique PID associated. Which will make the firmware name like
> > > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load
> > > at no flash condition. Thus PID information is required in dts when
> > > no-flash-flag
> > > is specified.
> >
> > Firstly, where does the "xxx" come from?
> > And you're making it sound more like having firmware-name is suitable
> > for this use case, given you need to determine the name of the file to
> > use based on something that is hardware specific but is not
> > dynamically detectable.
> Current driver patch uses a prefix name "himax_i2chid" which comes
> from the previous project
> and seems not suitable for this condition, so I use "xxx" and plan to
> replace it in the next version.
> For finding firmware, I think both solutions are reasonable.
> - provide firmware name directly: implies no-flash and use user
> specified firmware, no PID info.
> - provide no-flash-flag and PID info: loading firmware from organized
> names with PID info.
> I prefer the 2nd solution, but it needs more properties in dts. 1st
> has less properties and more
> intuitive.
>
> I don't know which one is more acceptable by the community, as you
> know I'm a newbie here.

To be honest, I am not all that sure either! Does the panel id have
value in its own right, or is that only used to determine the firmware
filename?
Also, if it does have value in its own right, rather than a "pid",
should the panel be a child node of this hid device with its own
compatible?


Attachments:
(No filename) (5.86 kB)
signature.asc (235.00 B)
Download all attachments

2023-10-02 17:07:03

by yang tylor

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

On Fri, Sep 29, 2023 at 12:56 AM Conor Dooley <[email protected]> wrote:
>
> On Thu, Sep 28, 2023 at 10:12:41AM +0800, yang tylor wrote:
> > On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <[email protected]> wrote:
> > > On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote:
> > > > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <[email protected]> wrote:
> > > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:
> > > > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <[email protected]> wrote:
> > > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.
> > > > >
> > > > > How do you intend generating the name of the firmware file? I assume the
> > > > > same firmware doesn't work on every IC, so you'll need to pick a
> > > > > different one depending on the compatible?
> > > > >
> > > > If considering a firmware library line-up for all the incoming panels
> > > > of this driver.
> > > > We would use PID as part of the file name. Because all the support panels would
> > > > have a unique PID associated. Which will make the firmware name like
> > > > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load
> > > > at no flash condition. Thus PID information is required in dts when
> > > > no-flash-flag
> > > > is specified.
> > >
> > > Firstly, where does the "xxx" come from?
> > > And you're making it sound more like having firmware-name is suitable
> > > for this use case, given you need to determine the name of the file to
> > > use based on something that is hardware specific but is not
> > > dynamically detectable.
> > Current driver patch uses a prefix name "himax_i2chid" which comes
> > from the previous project
> > and seems not suitable for this condition, so I use "xxx" and plan to
> > replace it in the next version.
> > For finding firmware, I think both solutions are reasonable.
> > - provide firmware name directly: implies no-flash and use user
> > specified firmware, no PID info.
> > - provide no-flash-flag and PID info: loading firmware from organized
> > names with PID info.
> > I prefer the 2nd solution, but it needs more properties in dts. 1st
> > has less properties and more
> > intuitive.
> >
> > I don't know which one is more acceptable by the community, as you
> > know I'm a newbie here.
>
> To be honest, I am not all that sure either! Does the panel id have
> value in its own right, or is that only used to determine the firmware
> filename?
Currently, PID stands for Panel/Project ID and is used for determining
the firmware filename only. We haven't come up with any new attribute that
may attach to it. The differences between panels are handled in firmware
dedicated to its PID.

> Also, if it does have value in its own right, rather than a "pid",
> should the panel be a child node of this hid device with its own
> compatible?
It may need a child node if we find it necessary to add attributes to each PID.
But currently we have no idea about it.

Thanks,
Tylor

2023-10-09 17:52:46

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

On Mon, Oct 02, 2023 at 06:44:41PM +0800, yang tylor wrote:
> On Fri, Sep 29, 2023 at 12:56 AM Conor Dooley <[email protected]> wrote:
> >
> > On Thu, Sep 28, 2023 at 10:12:41AM +0800, yang tylor wrote:
> > > On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <[email protected]> wrote:
> > > > On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote:
> > > > > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <[email protected]> wrote:
> > > > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:
> > > > > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <[email protected]> wrote:
> > > > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.
> > > > > >
> > > > > > How do you intend generating the name of the firmware file? I assume the
> > > > > > same firmware doesn't work on every IC, so you'll need to pick a
> > > > > > different one depending on the compatible?
> > > > > >
> > > > > If considering a firmware library line-up for all the incoming panels
> > > > > of this driver.
> > > > > We would use PID as part of the file name. Because all the support panels would
> > > > > have a unique PID associated. Which will make the firmware name like
> > > > > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load
> > > > > at no flash condition. Thus PID information is required in dts when
> > > > > no-flash-flag
> > > > > is specified.
> > > >
> > > > Firstly, where does the "xxx" come from?
> > > > And you're making it sound more like having firmware-name is suitable
> > > > for this use case, given you need to determine the name of the file to
> > > > use based on something that is hardware specific but is not
> > > > dynamically detectable.
> > > Current driver patch uses a prefix name "himax_i2chid" which comes
> > > from the previous project
> > > and seems not suitable for this condition, so I use "xxx" and plan to
> > > replace it in the next version.
> > > For finding firmware, I think both solutions are reasonable.
> > > - provide firmware name directly: implies no-flash and use user
> > > specified firmware, no PID info.
> > > - provide no-flash-flag and PID info: loading firmware from organized
> > > names with PID info.
> > > I prefer the 2nd solution, but it needs more properties in dts. 1st
> > > has less properties and more
> > > intuitive.
> > >
> > > I don't know which one is more acceptable by the community, as you
> > > know I'm a newbie here.
> >
> > To be honest, I am not all that sure either! Does the panel id have
> > value in its own right, or is that only used to determine the firmware
> > filename?
> Currently, PID stands for Panel/Project ID and is used for determining
> the firmware filename only. We haven't come up with any new attribute that
> may attach to it. The differences between panels are handled in firmware
> dedicated to its PID.
>
> > Also, if it does have value in its own right, rather than a "pid",
> > should the panel be a child node of this hid device with its own
> > compatible?
> It may need a child node if we find it necessary to add attributes to each PID.
> But currently we have no idea about it.

To be honest, it seems to me like you are using "PID" in place of a
compatible for the panel, since it needs to be provided via DT anyway.


Attachments:
(No filename) (3.29 kB)
signature.asc (235.00 B)
Download all attachments

2023-10-12 02:30:36

by yang tylor

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

On Tue, Oct 10, 2023 at 1:52 AM Conor Dooley <[email protected]> wrote:
>
> On Mon, Oct 02, 2023 at 06:44:41PM +0800, yang tylor wrote:
> > On Fri, Sep 29, 2023 at 12:56 AM Conor Dooley <[email protected]> wrote:
> > >
> > > On Thu, Sep 28, 2023 at 10:12:41AM +0800, yang tylor wrote:
> > > > On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <[email protected]> wrote:
> > > > > On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote:
> > > > > > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <[email protected]> wrote:
> > > > > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:
> > > > > > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <[email protected]> wrote:
> > > > > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.
> > > > > > >
> > > > > > > How do you intend generating the name of the firmware file? I assume the
> > > > > > > same firmware doesn't work on every IC, so you'll need to pick a
> > > > > > > different one depending on the compatible?
> > > > > > >
> > > > > > If considering a firmware library line-up for all the incoming panels
> > > > > > of this driver.
> > > > > > We would use PID as part of the file name. Because all the support panels would
> > > > > > have a unique PID associated. Which will make the firmware name like
> > > > > > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load
> > > > > > at no flash condition. Thus PID information is required in dts when
> > > > > > no-flash-flag
> > > > > > is specified.
> > > > >
> > > > > Firstly, where does the "xxx" come from?
> > > > > And you're making it sound more like having firmware-name is suitable
> > > > > for this use case, given you need to determine the name of the file to
> > > > > use based on something that is hardware specific but is not
> > > > > dynamically detectable.
> > > > Current driver patch uses a prefix name "himax_i2chid" which comes
> > > > from the previous project
> > > > and seems not suitable for this condition, so I use "xxx" and plan to
> > > > replace it in the next version.
> > > > For finding firmware, I think both solutions are reasonable.
> > > > - provide firmware name directly: implies no-flash and use user
> > > > specified firmware, no PID info.
> > > > - provide no-flash-flag and PID info: loading firmware from organized
> > > > names with PID info.
> > > > I prefer the 2nd solution, but it needs more properties in dts. 1st
> > > > has less properties and more
> > > > intuitive.
> > > >
> > > > I don't know which one is more acceptable by the community, as you
> > > > know I'm a newbie here.
> > >
> > > To be honest, I am not all that sure either! Does the panel id have
> > > value in its own right, or is that only used to determine the firmware
> > > filename?
> > Currently, PID stands for Panel/Project ID and is used for determining
> > the firmware filename only. We haven't come up with any new attribute that
> > may attach to it. The differences between panels are handled in firmware
> > dedicated to its PID.
> >
> > > Also, if it does have value in its own right, rather than a "pid",
> > > should the panel be a child node of this hid device with its own
> > > compatible?
> > It may need a child node if we find it necessary to add attributes to each PID.
> > But currently we have no idea about it.
>
> To be honest, it seems to me like you are using "PID" in place of a
> compatible for the panel, since it needs to be provided via DT anyway.

Hmm... So the more formal way is?
If I add a sub-note inside this spi-device block, such as "panel" and
add PID inside.
Will it be more appropriate?
...
spi {
...
hx_spi@0 {
...
panel {
himax,pid = ...
};
}
}

Thanks,
Tylor

2023-10-12 15:25:19

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

On Thu, Oct 12, 2023 at 10:30:03AM +0800, yang tylor wrote:
> On Tue, Oct 10, 2023 at 1:52 AM Conor Dooley <[email protected]> wrote:
> >
> > On Mon, Oct 02, 2023 at 06:44:41PM +0800, yang tylor wrote:
> > > On Fri, Sep 29, 2023 at 12:56 AM Conor Dooley <[email protected]> wrote:
> > > >
> > > > On Thu, Sep 28, 2023 at 10:12:41AM +0800, yang tylor wrote:
> > > > > On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <[email protected]> wrote:
> > > > > > On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote:
> > > > > > > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <[email protected]> wrote:
> > > > > > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:
> > > > > > > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <[email protected]> wrote:
> > > > > > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.
> > > > > > > >
> > > > > > > > How do you intend generating the name of the firmware file? I assume the
> > > > > > > > same firmware doesn't work on every IC, so you'll need to pick a
> > > > > > > > different one depending on the compatible?
> > > > > > > >
> > > > > > > If considering a firmware library line-up for all the incoming panels
> > > > > > > of this driver.
> > > > > > > We would use PID as part of the file name. Because all the support panels would
> > > > > > > have a unique PID associated. Which will make the firmware name like
> > > > > > > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load
> > > > > > > at no flash condition. Thus PID information is required in dts when
> > > > > > > no-flash-flag
> > > > > > > is specified.
> > > > > >
> > > > > > Firstly, where does the "xxx" come from?
> > > > > > And you're making it sound more like having firmware-name is suitable
> > > > > > for this use case, given you need to determine the name of the file to
> > > > > > use based on something that is hardware specific but is not
> > > > > > dynamically detectable.
> > > > > Current driver patch uses a prefix name "himax_i2chid" which comes
> > > > > from the previous project
> > > > > and seems not suitable for this condition, so I use "xxx" and plan to
> > > > > replace it in the next version.
> > > > > For finding firmware, I think both solutions are reasonable.
> > > > > - provide firmware name directly: implies no-flash and use user
> > > > > specified firmware, no PID info.
> > > > > - provide no-flash-flag and PID info: loading firmware from organized
> > > > > names with PID info.
> > > > > I prefer the 2nd solution, but it needs more properties in dts. 1st
> > > > > has less properties and more
> > > > > intuitive.
> > > > >
> > > > > I don't know which one is more acceptable by the community, as you
> > > > > know I'm a newbie here.
> > > >
> > > > To be honest, I am not all that sure either! Does the panel id have
> > > > value in its own right, or is that only used to determine the firmware
> > > > filename?
> > > Currently, PID stands for Panel/Project ID and is used for determining
> > > the firmware filename only. We haven't come up with any new attribute that
> > > may attach to it. The differences between panels are handled in firmware
> > > dedicated to its PID.
> > >
> > > > Also, if it does have value in its own right, rather than a "pid",
> > > > should the panel be a child node of this hid device with its own
> > > > compatible?
> > > It may need a child node if we find it necessary to add attributes to each PID.
> > > But currently we have no idea about it.
> >
> > To be honest, it seems to me like you are using "PID" in place of a
> > compatible for the panel, since it needs to be provided via DT anyway.
>
> Hmm... So the more formal way is?
> If I add a sub-note inside this spi-device block, such as "panel" and
> add PID inside.
> Will it be more appropriate?
> ...
> spi {
> ...
> hx_spi@0 {
> ...
> panel {
> himax,pid = ...

And this now looks exactly like compatible = "vendor,part" now, no?


Attachments:
(No filename) (4.00 kB)
signature.asc (235.00 B)
Download all attachments

2023-10-13 02:15:56

by yang tylor

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

On Thu, Oct 12, 2023 at 11:24 PM Conor Dooley <[email protected]> wrote:
>
> On Thu, Oct 12, 2023 at 10:30:03AM +0800, yang tylor wrote:
> > On Tue, Oct 10, 2023 at 1:52 AM Conor Dooley <[email protected]> wrote:
> > >
> > > On Mon, Oct 02, 2023 at 06:44:41PM +0800, yang tylor wrote:
> > > > On Fri, Sep 29, 2023 at 12:56 AM Conor Dooley <[email protected]> wrote:
> > > > >
> > > > > On Thu, Sep 28, 2023 at 10:12:41AM +0800, yang tylor wrote:
> > > > > > On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <[email protected]> wrote:
> > > > > > > On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote:
> > > > > > > > On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley <[email protected]> wrote:
> > > > > > > > > On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:
> > > > > > > > > > On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley <[email protected]> wrote:
> > > > > > > > > > We have a default prefix firmware name(like himax_xxxx.bin) in the driver code.
> > > > > > > > >
> > > > > > > > > How do you intend generating the name of the firmware file? I assume the
> > > > > > > > > same firmware doesn't work on every IC, so you'll need to pick a
> > > > > > > > > different one depending on the compatible?
> > > > > > > > >
> > > > > > > > If considering a firmware library line-up for all the incoming panels
> > > > > > > > of this driver.
> > > > > > > > We would use PID as part of the file name. Because all the support panels would
> > > > > > > > have a unique PID associated. Which will make the firmware name like
> > > > > > > > himax_xxx_{$PID}.bin. The problem is, we need to know PID before firmware load
> > > > > > > > at no flash condition. Thus PID information is required in dts when
> > > > > > > > no-flash-flag
> > > > > > > > is specified.
> > > > > > >
> > > > > > > Firstly, where does the "xxx" come from?
> > > > > > > And you're making it sound more like having firmware-name is suitable
> > > > > > > for this use case, given you need to determine the name of the file to
> > > > > > > use based on something that is hardware specific but is not
> > > > > > > dynamically detectable.
> > > > > > Current driver patch uses a prefix name "himax_i2chid" which comes
> > > > > > from the previous project
> > > > > > and seems not suitable for this condition, so I use "xxx" and plan to
> > > > > > replace it in the next version.
> > > > > > For finding firmware, I think both solutions are reasonable.
> > > > > > - provide firmware name directly: implies no-flash and use user
> > > > > > specified firmware, no PID info.
> > > > > > - provide no-flash-flag and PID info: loading firmware from organized
> > > > > > names with PID info.
> > > > > > I prefer the 2nd solution, but it needs more properties in dts. 1st
> > > > > > has less properties and more
> > > > > > intuitive.
> > > > > >
> > > > > > I don't know which one is more acceptable by the community, as you
> > > > > > know I'm a newbie here.
> > > > >
> > > > > To be honest, I am not all that sure either! Does the panel id have
> > > > > value in its own right, or is that only used to determine the firmware
> > > > > filename?
> > > > Currently, PID stands for Panel/Project ID and is used for determining
> > > > the firmware filename only. We haven't come up with any new attribute that
> > > > may attach to it. The differences between panels are handled in firmware
> > > > dedicated to its PID.
> > > >
> > > > > Also, if it does have value in its own right, rather than a "pid",
> > > > > should the panel be a child node of this hid device with its own
> > > > > compatible?
> > > > It may need a child node if we find it necessary to add attributes to each PID.
> > > > But currently we have no idea about it.
> > >
> > > To be honest, it seems to me like you are using "PID" in place of a
> > > compatible for the panel, since it needs to be provided via DT anyway.
> >
> > Hmm... So the more formal way is?
> > If I add a sub-note inside this spi-device block, such as "panel" and
> > add PID inside.
> > Will it be more appropriate?
> > ...
> > spi {
> > ...
> > hx_spi@0 {
> > ...
> > panel {
> > himax,pid = ...
>
> And this now looks exactly like compatible = "vendor,part" now, no?

I think it's not the same, I thought "compatible" is used to target
from the driver side.
For finding other information inside the block. But I just store PID
information in this
one, not used for targeting but getting infos from it.

Thanks,
Tylor