On 11/05/2023 13:26, Aidan MacDonald wrote: > > Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> writes: > >> On 11/05/2023 12:15, Aidan MacDonald wrote: >>> >>> Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> writes: >>> >>>> On 10/05/2023 13:23, Aidan MacDonald wrote: >>>>> ... >>>>> + ess,max-clock-div: >>>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>>> + description: >>>>> + Sets the maximum MCLK divider for generating the internal CLK. >>>>> + CLK must be at least 20x the I2C bus speed or I2C transactions >>>>> + will fail. The maximum divider should be chosen to ensure that >>>>> + CLK will not fall below the limit. >>>>> + enum: >>>>> + - 1 >>>>> + - 2 >>>>> + - 4 >>>>> + - 8 >>>>> + default: 1 >>>> >>>> Why do you need to customize it per board? >>>> >>> >>> There's no generic API to read or change the bus speed (SCL frequency) >>> for I2C adapters, so it's impossible to calculate a limit on the MCLK >>> divider automatically. >>> >>> Defaulting to 1 is always safe, but means wasting power at lower sample >>> rates. If you know what the bus speed is you can use a higher divider >>> limit to save power, and it has to be done at the board/firmware level >>> because that's the only place where the bus speed is known. >> >> If I understand correctly, you only miss a way to get bus_freq_hz from >> i2c_timings to calculate the divider by yourself? This looks like Linux >> limitation, so we shouldn't push it into DT, but rather fix Linux. The >> I2C bus rate is known, the MCLK rate as well, so divider is possible to >> deduce. >> >> I am actually surprised that I2C core does not store the timings >> anywhere and each bus host has to deal with it on its own. >> >> Best regards, >> Krzysztof > > I agree it'd be better if Linux provided access the bus frequency, but > even if that API was added it will take time for every I2C adapter to > support it. So in that case we would still need a DT property to provide > a safe limit or use a safe default, and miss potential power savings. > > I'd prefer to add the DT property to allow power savings to be had now, > and drop it if/when the kernel gets an API for bus frequency. That will > be safe from a compatibility point of view -- the property won't be > providing any useful information so it won't matter if old DTs have it. OK, sounds good. Best regards, Krzysztof