On 08/12/21 05:45PM, Dinh Nguyen wrote: > On Mon, Dec 6, 2021 at 9:51 PM Pratyush Yadav <p.yadav@xxxxxx> wrote: > > > > On 03/12/21 12:17PM, Dinh Nguyen wrote: > > > The QSPI controller on Intel's SoCFPGA platform does not implement the > > > CQSPI_REG_WR_COMPLETION_CTRL register, thus a write to this register > > > results in a crash. > > > > > > The module/revision ID is written in the MODULE_ID register. For this > > > variance, bits 23-8 is 0x0010. > > > > When I looked at your original patches I was under the impression that > > this was a SoCFPGA specific thing and did not apply to other > > implementation of the IP in general. > > > > If this is indeed a generic thing and we can detect it via the MODULE_ID > > register [0], then why not just read that register at probe time and > > apply this quirk based on the ID? Why then do we need a separate > > compatible at all? > > > > [0] I would like to see it stated explicitly somewhere that version > > 0x0010 does not support the WR_COMPLETION_CTRL register. > > > > I cannot for sure confirm that this condition applies to only 0x0010 > version of the > IP. I can verify that the IP that is in all 3 generations of SoCFPGA > devices, all have > MODULE_ID value of 0x0010 and all do not have the WR_COMPLETION_CTRL > register implemented. I agree with Rob here. If you are not sure that this is a generic IP thing then you should not use a generic compatible. > > I'm almost certain this feature is not SoCFPGA specific, but > since I only had SoCFPGA hardware, that was my initial patch. I made the mistake > of not CC'ing the devicetree maintainers until I sent the DTS binding > patch change, > and he rightly suggested making the binding something more generic. > > I do like your idea of making a determination in the driver without > being dependent > on a dts binding. I'd like to know the impetus behind your original > patch of removing the > dependency of "if (f_pdata->dtr)" for the write to the WR_COMPLETION_CTRL > register? Perhaps there's some other common property that we can key > off for why the register > is not implemented? Please read the comment just above that line ;-) /* * SPI NAND flashes require the address of the status register to be * passed in the Read SR command. Also, some SPI NOR flashes like the * cypress Semper flash expect a 4-byte dummy address in the Read SR * command in DTR mode. * * But this controller does not support address phase in the Read SR * command when doing auto-HW polling. So, disable write completion * polling on the controller's side. spinand and spi-nor will take * care of polling the status register. */ -- Regards, Pratyush Yadav Texas Instruments Inc.
