On Tue, Oct 10, 2023 at 1:52 AM Conor Dooley [off-list ref] wrote:

On Mon, Oct 02, 2023 at 06:44:41PM +0800, yang tylor wrote:

quoted

On Fri, Sep 29, 2023 at 12:56 AM Conor Dooley [off-list ref] wrote:

quoted

On Thu, Sep 28, 2023 at 10:12:41AM +0800, yang tylor wrote:

quoted

On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley [off-list ref] wrote:

quoted

On Tue, Sep 26, 2023 at 05:52:39PM +0800, yang tylor wrote:

quoted

On Tue, Sep 26, 2023 at 5:02 PM Conor Dooley [off-list ref] wrote:

quoted

On Mon, Sep 25, 2023 at 06:16:29PM +0800, yang tylor wrote:

quoted

On Mon, Sep 25, 2023 at 4:41 PM Conor Dooley [off-list ref] 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.

quoted

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