Hi Wolfram, Thanks for your feedback. On 2018-11-29 17:54:34 +0100, Wolfram Sang wrote: > Hi Niklas, > > thanks for the patches! > > On Thu, Nov 29, 2018 at 01:39:49AM +0100, Niklas Söderlund wrote: > > On H3 (ES1.x, ES2.0) and M3-W (ES1.0, ES1.1) the clock setting for HS400 > > needs a quirk to function properly. The reason for the quirk is that > > there are two settings which produces same divider value for the SDn > > clock. On the effected boards the one currently selected results in > > HS400 not working. > > > > This change uses the same method as the Gen2 CPG driver and simply > > ignores the first clock setting as this is the offending one when > > selecting the settings. Which of the two possible settings is used have > > no effect for SDR104. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx> > > > > --- > > * Changes since v1 > > - Fixed spelling in commit message, thanks Sergei and Geert! > > - Reworked the whole patch per Geerts suggestion. Instead of only > > skipping the first row on the effected boards when setting the clock > > rete totally ignore it. This is made possible by another change to the > > "rete"? I don't get this sentence and I think it is important to > understand when reviewing these patches :) That should have been rate :-) To elaborate a bit more: The patch is different from v1 as a different approach to solve the issue have been found. Instead of only ignoring the first row of the list of possible settings when selecting which divider to use also ignore it when examining which state the hardware is in. That is the driver is no longer aware the first row exists with this patch. This was in v1 not possible as the first row might be a state the bootloader left the hardware in and then the clock failed to register as it would need to update its own state to match the hardware. As the driver needed to know about the state the hardware was in when probing but not use it when selecting a divider the more complex v1 was needed. When selecting a divider we wish for it to select the second option for the divider value '4' when running on a SoC which needs the quirk. With v2 which depends on [1] this is not needed as the clock driver now sets a know state when registering the clock so this patch can be made much simpler by simply 'removing' the first row from all operations. > > > clock driver posted separately from this series and which this patch > > now depends on [1]. > > Hmm, why didn't you add it to the series then? Since it is unrelated to this series I thought it best to post it as a separate patch as I think it has value to create a known starting state disregarding where this series ends up :-) > > Still, all in all, seems we are on a nice track for having HS400 in the > next release \o/ Now, if that doesn't justify the 5.0 jump... ;D I sure hope so! -- Regards, Niklas Söderlund