On Tue, Sep 26, 2023 at 8:53 PM Conor Dooley <conor@xxxxxxxxxx> 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 <conor@xxxxxxxxxx> 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 <conor.dooley@xxxxxxxxxxxxx> 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 <conor@xxxxxxxxxx> 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 <conor@xxxxxxxxxx> 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 <conor@xxxxxxxxxx> 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