> Can I be awkward and ask that we hold off on this patch chain until > then? Then we can add this laptop using the new approach. > If/when the chain is accepted, we will add support for a few Dell > laptops as well, including this one. Sounds reasonable. I'll be looking forward to your new framework. Once up, I can adjust my patch, and if everything still works as expected, push updated version for review. > Instead of erroring out, I wonder if we can noodle our way to the > appropriate clk and clk_set_rate it up to 4MHz for this particular > laptop only? Stefan's taking a look at that. Thanks for the initiative. Potentially that would work, however, it would require to go up the clock tree to the divider. Since its clearly a firmware bug causing lpss miss-configuration, I intially thought it would be best to have it resolved there. If you need more information, I would be happy to share results of our debugging with you via private email. > Also, any SPI rate >~100k is probably just about usable, so we don't > want to error on <4MHz. Quite often the spi clock is set at some value > just below 4MHz. It's unclear if this is going to get fixed in the BIOS > at this point, so we don't know what exact rate we'd eventually receive. I'm afraid I have to disagree here, 100k is _way_ too slow. Not sure intentionally or not, but wake up from suspend is held back by Cirrus driver. At 100k, I got these results, on boot: ``` [ 5.561244] cs35l41-hda spi1-CSC3551:00-cs35l41-hda.0: Adding DSD properties for 10280BEB .. [ 11.251145] cs35l41-hda spi1-CSC3551:00-cs35l41-hda.1: CS35L41 Bound - SSID: 10280BEB, BST: 1, VSPK: 1, CH: R, FW EN: 1, SPKID: -19 ``` And on wake-up from suspend: ``` [ 307.162720] cs35l41-hda spi1-CSC3551:00-cs35l41-hda.0: DSP1: Firmware version: 3 ... [ 312.515588] cs35l41-hda spi1-CSC3551:00-cs35l41-hda.1: 100000 Hz actual, DMA ``` This means ~5.5 additional seconds of black screen on wake up, in my opnion this is completely unacceptable. With 4Mhz, it takes sub 1second. Moreover, the first time (after preconfigured by ALSA delay) sound is played, it seems it needs to communicate with amplifier, and it takes additional few seconds to start playing at 100Khz. With 4Mhz, its practically instant. I agree that it is unlikely for Dell to ever fix its firmware. Thus either in case of intel-lpss patch, or via clk_set_rate direclty from Cirrus driver, lpss divider would be set 1:1, SPI controller would receive 100Mhz clock directly, and set it to whatever is requested internally (currently 4Mhz). This way, lowest 'usable' rate should be irrelevant. > Quite often the spi clock is set at some value just below 4MHz Perhaps to address this, we could error out on say half requested rate? In reality, it will be either requested rate/just sub requested, or something totally off, like the default 3051Hz in this case. > This will break once you pick up AMD's multi-cs patches, we should use > spi_set_csgpiod instead. Thanks, will correct. > I suppose the error-out was due to safety reasons, but the clock > adjustment works, it should be fine. Let's see. Precisely, since it is indeed unknown when exaclty/if ever firmware will be fixed, and/or when our patch to intel-lpss will be accepted. Regards, Alex
