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: > > Hi Conor, > > > > > > Additional optional arguments: > > > > ic-det-delay-ms and ic-resume-delay-ms are using to solve runtime > > > > conditions. > > > > > Runtime conditions? Aren't thєse properties of the panel & therefore > > > fixed? If they were runtime conditions, then setting them statically in > > > your DT is not going to work, right? > > > > Because each platform's display driver ready time is different. TP part > > need to avoid this timing by measuring the waveform of LCD reset pin > > low period and TP probe timing. For example, if LCD rst pin low from > > timestamp 100 to 800, TP driver probe at 600. TP probe will fail. Then > > user should set ic-det-delay-ms bigger than 200, to avoid LCD rst low > > timing. As you can see, the timing needs to be measured at runtime to > > decide how long it should be. Then, if the condition is not changed, the > > value could keep the same. > > That sounds to me like something you would test once for a given > platform and then the values are static. If you are actually changing it > at *runtime*, how is doing it through DT suitable? Does your firmware do > the tests & then set the values in DT dynamically? > Yes, you are right. I'll change the description. > > > > > It looks like you deleted all of the properties from the previous > > > submission of these changes. I don't really understand that, it kinda > > > feels just like appeasement, as you must have needed those properties > > > to do the firmware loading etc. How are you filling the gap those > > > properties have left, when you still only have a single compatible > > > string in thㄟs binding? Is there a way to do runtime detection of which > > > chip you're dealing with that you are now using? > > > > After reviewing, I found the properties could go to IC driver settings : > > "himax,heatmap_16bits" because it depends on IC's ability; > > How do you detect the IC's abilities? > The driver code has a part of IC detect process, and each IC has its own driver code to define its abilities. This part moves to that position. > > Some > > could remove and use default values: "himax,fw_size", > > "himax,boot_time_fw_upgrade". "himax,fw_size" has a default value in > > IC settings, and likely won't change in this IC. > > Okay. > > > 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. > > "himax,pid" could be remove and use default firmware name > > "himax_i2chid.bin" to load. It was added because users may desire to > > choose a special name like "himax_i2chid_{pid}.bin" instead of the default > > one. > > It also could be replaced with newly added "himax",id-gpios" which is still > > experimental. > > Also, pleae don't top post, but instead reply in-line with my comments, > as I have done here. > Ok. > > Btw, I encounter an error of patch [2/2], which says: > > BOUNCE linux-input@xxxxxxxxxxxxxxx: Message too long (>100000 chars) > > and the patch didn't appear at patchwork.kernel.org. What should I do to > > deal with this problem? > > No idea. Maybe try to split it into multiple patches? > The other option is to also cc patches@xxxxxxxxxxxxxxx as that has some > higher capacities, but that's not going to be a silver bullet. Thanks for the reply. I'll try multiple commits to reduce the size. Thanks, Tylor