| Message ID | 20211112204927.8830-5-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | spi: dw: Cleanup macros/funcs naming and add IP-core version support | expand |
On Fri, Nov 12, 2021 at 10:52 PM Serge Semin <Sergey.Semin@baikalelectronics.ru> wrote: > > Each Synopsys DWC SSI controller is equipped with a Synopsys Component > version register, which encodes a version ID of an IP-core the > controller has been synthesized from. That can be useful in future for the > version-based conditional features implementation in the driver. > > Note the component version is encoded as an ASCII string so we need to > convert it from the string to a normal unsigned integer to be easily used > then in the driver statements. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > --- > drivers/spi/spi-dw-core.c | 18 ++++++++++++++++++ > drivers/spi/spi-dw.h | 1 + > 2 files changed, 19 insertions(+) > > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c > index b4cbcd38eaba..1766a29ca790 100644 > --- a/drivers/spi/spi-dw-core.c > +++ b/drivers/spi/spi-dw-core.c > @@ -823,6 +823,24 @@ static void dw_spi_hw_init(struct device *dev, struct dw_spi *dws) > { > dw_spi_reset_chip(dws); > > + /* > + * Retrieve the Synopsys component version if it hasn't been specified > + * by the platform. Note the CoreKit version ID is encoded as a 4bytes > + * ASCII string enclosed with '*' symbol. > + */ > + if (!dws->ver) { > + u32 comp; > + > + comp = dw_readl(dws, DW_SPI_VERSION); > + dws->ver = (DW_SPI_GET_BYTE(comp, 3) - '0') * 100; > + dws->ver += (DW_SPI_GET_BYTE(comp, 2) - '0') * 10; > + dws->ver += (DW_SPI_GET_BYTE(comp, 1) - '0'); > + > + dev_dbg(dev, "Synopsys DWC%sSSI v%u.%02u\n", > + (dws->caps & DW_SPI_CAP_DWC_HSSI) ? " " : " APB ", > + dws->ver / 100, dws->ver % 100); Oh là là, first you multiply then you divide in the same piece of code! What's wrong with fourcc (and thus keep it in ver filed as is) ? (Also we have %p4cc) > + } > + > /* > * Try to detect the FIFO depth if not set by interface driver, > * the depth could be from 2 to 256 from HW spec > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h > index 634085eadad1..d06857d8d173 100644 > --- a/drivers/spi/spi-dw.h > +++ b/drivers/spi/spi-dw.h > @@ -149,6 +149,7 @@ struct dw_spi { > u32 max_mem_freq; /* max mem-ops bus freq */ > u32 max_freq; /* max bus freq supported */ > > + u32 ver; /* Synopsys component version */ > u32 caps; /* DW SPI capabilities */ > > u32 reg_io_width; /* DR I/O width in bytes */ > -- > 2.33.0 >
On Fri, Nov 12, 2021 at 11:27 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Fri, Nov 12, 2021 at 10:52 PM Serge Semin > <Sergey.Semin@baikalelectronics.ru> wrote: > > + /* > > + * Retrieve the Synopsys component version if it hasn't been specified > > + * by the platform. Note the CoreKit version ID is encoded as a 4bytes > > + * ASCII string enclosed with '*' symbol. > > + */ > > + if (!dws->ver) { > > + u32 comp; > > + > > + comp = dw_readl(dws, DW_SPI_VERSION); > > + dws->ver = (DW_SPI_GET_BYTE(comp, 3) - '0') * 100; > > + dws->ver += (DW_SPI_GET_BYTE(comp, 2) - '0') * 10; > > + dws->ver += (DW_SPI_GET_BYTE(comp, 1) - '0'); > > + > > + dev_dbg(dev, "Synopsys DWC%sSSI v%u.%02u\n", > > + (dws->caps & DW_SPI_CAP_DWC_HSSI) ? " " : " APB ", > > + dws->ver / 100, dws->ver % 100); > > Oh là là, first you multiply then you divide in the same piece of code! > What's wrong with fourcc (and thus keep it in ver filed as is) ? (Also > we have %p4cc) > > > + } Have you seen this, btw? https://elixir.bootlin.com/linux/latest/source/drivers/tty/serial/8250/8250_dwlib.c#L93
On Fri, Nov 12, 2021 at 11:37:36PM +0200, Andy Shevchenko wrote: > On Fri, Nov 12, 2021 at 11:27 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Fri, Nov 12, 2021 at 10:52 PM Serge Semin > > <Sergey.Semin@baikalelectronics.ru> wrote: > > > > + /* > > > + * Retrieve the Synopsys component version if it hasn't been specified > > > + * by the platform. Note the CoreKit version ID is encoded as a 4bytes > > > + * ASCII string enclosed with '*' symbol. > > > + */ > > > + if (!dws->ver) { > > > + u32 comp; > > > + > > > + comp = dw_readl(dws, DW_SPI_VERSION); > > > + dws->ver = (DW_SPI_GET_BYTE(comp, 3) - '0') * 100; > > > + dws->ver += (DW_SPI_GET_BYTE(comp, 2) - '0') * 10; > > > + dws->ver += (DW_SPI_GET_BYTE(comp, 1) - '0'); > > > + > > > + dev_dbg(dev, "Synopsys DWC%sSSI v%u.%02u\n", > > > + (dws->caps & DW_SPI_CAP_DWC_HSSI) ? " " : " APB ", > > > + dws->ver / 100, dws->ver % 100); > > > > Oh là là, first you multiply then you divide in the same piece of code! > > What's wrong with fourcc (and thus keep it in ver filed as is) ? (Also > > we have %p4cc) Please note that's just a dev_DBG() print. So division has been used in there to check whether the conversion was correct. The whole idea behind the code above it was to retrieve the Component version as a single number so then it could be used by the driver code in a simple statement with a normal integer operation. For instance in case if we need to check whether DW SSI IP-core version is greater than 1.01 we'd have something like this: if (dws->ver > 101). Here 101 looks at least close to the original 1.01. How would the statement look with four chars? Of course we could add an another macro which would look like this: #define DW_SSI_VER(_maj, _mid, _min) \ ((_maj) << 24 | (_mid) << 16 | (_min) << 8 | '*') and use it with raw version ID, like this (dws->ver > DW_SSI_VER('1', '0', '1')). But IMO it doesn't look better if not worse. Alternatively we could split the version ID into two parts with major and minor numbers. But normally one doesn't make much sense without another so each statement would need to check both of them at once anyway. So I decided to stick with a simplest solution and combined them into a single storage. Have you got a better idea of how to implement this functionality? > > > > > + } > > Have you seen this, btw? > > https://elixir.bootlin.com/linux/latest/source/drivers/tty/serial/8250/8250_dwlib.c#L93 It doesn't utilized version ID for something functional, but just prints it to the console. So it isn't that good reference in this case. -Sergey > > > -- > With Best Regards, > Andy Shevchenko
On Sat, Nov 13, 2021 at 01:14:30AM +0200, Andy Shevchenko wrote: > On Saturday, November 13, 2021, Serge Semin <fancer.lancer@gmail.com> wrote: > > > On Fri, Nov 12, 2021 at 11:37:36PM +0200, Andy Shevchenko wrote: > > > On Fri, Nov 12, 2021 at 11:27 PM Andy Shevchenko > > > <andy.shevchenko@gmail.com> wrote: > > > > On Fri, Nov 12, 2021 at 10:52 PM Serge Semin > > > > <Sergey.Semin@baikalelectronics.ru> wrote: > > > > > > > > + /* > > > > > + * Retrieve the Synopsys component version if it hasn't been > > specified > > > > > + * by the platform. Note the CoreKit version ID is encoded > > as a 4bytes > > > > > + * ASCII string enclosed with '*' symbol. > > > > > + */ > > > > > + if (!dws->ver) { > > > > > + u32 comp; > > > > > + > > > > > + comp = dw_readl(dws, DW_SPI_VERSION); > > > > > + dws->ver = (DW_SPI_GET_BYTE(comp, 3) - '0') * 100; > > > > > + dws->ver += (DW_SPI_GET_BYTE(comp, 2) - '0') * 10; > > > > > + dws->ver += (DW_SPI_GET_BYTE(comp, 1) - '0'); > > > > > + > > > > > + dev_dbg(dev, "Synopsys DWC%sSSI v%u.%02u\n", > > > > > + (dws->caps & DW_SPI_CAP_DWC_HSSI) ? " " : " > > APB ", > > > > > + dws->ver / 100, dws->ver % 100); > > > > > > > > > > Oh là là, first you multiply then you divide in the same piece of code! > > > > What's wrong with fourcc (and thus keep it in ver filed as is) ? (Also > > > > we have %p4cc) > > > > Please note that's just a dev_DBG() print. So division has been used > > in there to check whether the conversion was correct. The whole idea > > behind the code above it was to retrieve the Component version as a > > single number so then it could be used by the driver code in a simple > > statement with a normal integer operation. For instance in case if we > > need to check whether DW SSI IP-core version is greater than 1.01 we'd > > have something like this: if (dws->ver > 101). Here 101 looks at least > > close to the original 1.01. How would the statement look with four > > chars? Of course we could add an another macro which would look like > > this: > > #define DW_SSI_VER(_maj, _mid, _min) \ > > ((_maj) << 24 | (_mid) << 16 | (_min) << 8 | '*') > > and use it with raw version ID, like this > > (dws->ver > DW_SSI_VER('1', '0', '1')). But IMO it doesn't look > > better if not worse. > > Alternatively we could split the version ID into two parts with > > major and minor numbers. But normally one doesn't make much sense > > without another so each statement would need to check both of them > > at once anyway. So I decided to stick with a simplest solution and > > combined them into a single storage. Have you got a better idea of > > how to implement this functionality? > > > > Then check DWC3 driver which relies on IZp version a lot. I'm still not convinced that the DWC3 solution would be better in this case. (I had a similar approach in mind though.) Although it might be suitable here seeing we could take the IP generation into account in a single macro. But at the same time having macros defined for each version may eventually turn into a clumsy set of macros space as it happened in DWC3. I don't understand what do you see wrong in the suggested here solution except a math in the debug string? Why would you prefer the DWC3 approach better than the one implemented in my patch? I don't really see much benefits in it: if (dws->ver > 101) or if (DW_SPI_VER_AFTER(dws, 101)) In both cases version ID isn't represented in the original Vendor-defined structure, like "1.01". The only part which could be considered as better in DWC3 approach is having a macro name, which gives a bit better notion about the operation. But does it really worth introducing a new abstraction in the driver? On the other hand we could intermix the approaches. For instance decode the Component version as I suggested in this patch and implement a set of version checking macro. Thus we won't need so many additional macro encoding the SSI_COMP_VERSION content. If only we could have a macro like DW_SPI_VER(dws, >=, "1.01a") with no performance drawbacks I'd be glad to use it. AFAIU compiler can't operate with the string literal symbols, thus the symbols extraction like "1.01a"[0] will be performed on each statement execution which isn't that performant comparing to a simple two integers comparison. BTW note the DWC3 macros implicitly depend on having a local variable with dwc name which violates the kernel coding style. -Sergey > > > > > > > > > > > > > + } > > > > > > > > Have you seen this, btw? > > > > > > https://elixir.bootlin.com/linux/latest/source/drivers/ > > tty/serial/8250/8250_dwlib.c#L93 > > > > It doesn't utilized version ID for something functional, but just > > prints it to the console. So it isn't that good reference in this > > case. > > > > -Sergey > > > > > > > > > > > -- > > > With Best Regards, > > > Andy Shevchenko > > > > > -- > With Best Regards, > Andy Shevchenko
On Sat, Nov 13, 2021 at 2:19 AM Serge Semin <fancer.lancer@gmail.com> wrote: > On Sat, Nov 13, 2021 at 01:14:30AM +0200, Andy Shevchenko wrote: > > On Saturday, November 13, 2021, Serge Semin <fancer.lancer@gmail.com> wrote: > > > On Fri, Nov 12, 2021 at 11:37:36PM +0200, Andy Shevchenko wrote: > > > > On Fri, Nov 12, 2021 at 11:27 PM Andy Shevchenko > > > > <andy.shevchenko@gmail.com> wrote: > > > > > On Fri, Nov 12, 2021 at 10:52 PM Serge Semin > > > > > <Sergey.Semin@baikalelectronics.ru> wrote: ... > > > > > > + /* > > > > > > + * Retrieve the Synopsys component version if it hasn't been > > > specified > > > > > > + * by the platform. Note the CoreKit version ID is encoded > > > as a 4bytes > > > > > > + * ASCII string enclosed with '*' symbol. > > > > > > + */ > > > > > > + if (!dws->ver) { > > > > > > + u32 comp; > > > > > > + > > > > > > + comp = dw_readl(dws, DW_SPI_VERSION); > > > > > > + dws->ver = (DW_SPI_GET_BYTE(comp, 3) - '0') * 100; > > > > > > + dws->ver += (DW_SPI_GET_BYTE(comp, 2) - '0') * 10; > > > > > > + dws->ver += (DW_SPI_GET_BYTE(comp, 1) - '0'); > > > > > > + > > > > > > + dev_dbg(dev, "Synopsys DWC%sSSI v%u.%02u\n", > > > > > > + (dws->caps & DW_SPI_CAP_DWC_HSSI) ? " " : " > > > APB ", > > > > > > + dws->ver / 100, dws->ver % 100); > > > > > > > > > > > > > Oh lą lą, first you multiply then you divide in the same piece of code! > > > > > What's wrong with fourcc (and thus keep it in ver filed as is) ? (Also > > > > > we have %p4cc) > > > > > > Please note that's just a dev_DBG() print. So division has been used > > > in there to check whether the conversion was correct. The whole idea > > > behind the code above it was to retrieve the Component version as a > > > single number so then it could be used by the driver code in a simple > > > statement with a normal integer operation. For instance in case if we > > > need to check whether DW SSI IP-core version is greater than 1.01 we'd > > > have something like this: if (dws->ver > 101). Here 101 looks at least > > > close to the original 1.01. How would the statement look with four > > > chars? Of course we could add an another macro which would look like > > > this: > > > #define DW_SSI_VER(_maj, _mid, _min) \ > > > ((_maj) << 24 | (_mid) << 16 | (_min) << 8 | '*') > > > and use it with raw version ID, like this > > > (dws->ver > DW_SSI_VER('1', '0', '1')). But IMO it doesn't look > > > better if not worse. > > > Alternatively we could split the version ID into two parts with > > > major and minor numbers. But normally one doesn't make much sense > > > without another so each statement would need to check both of them > > > at once anyway. So I decided to stick with a simplest solution and > > > combined them into a single storage. Have you got a better idea of > > > how to implement this functionality? > > > Then check DWC3 driver which relies on IZp version a lot. > > I'm still not convinced that the DWC3 solution would be better in this case. > (I had a similar approach in mind though.) Although it might be suitable > here seeing we could take the IP generation into account in a single > macro. But at the same time having macros defined for each version may > eventually turn into a clumsy set of macros space as it happened in DWC3. > > I don't understand what do you see wrong in the suggested here > solution except a math in the debug string? The transformation ruins the fourcc [1] representation. We know that this is an ASCII. We know the position and meaning of each from 4 characters. [1]: https://en.wikipedia.org/wiki/FourCC > Why would you prefer the > DWC3 approach better than the one implemented in my patch? You asked what is _better_ implementation than yours. It doesn't mean the DWC3 approach fully fits here, but - SPI DW has the same issue as DWC3 solves, i.e. different version lines for two or more IPs of the same kind (see DWC3, DWC31, DWC32) - it doesn't ruins 4cc In case if we need to print it, I would rather see something in %p4cc implementation than customized 100500 approaches spreaded over the entire kernel. > I don't really see much benefits in it: > if (dws->ver > 101) > or > if (DW_SPI_VER_AFTER(dws, 101)) > In both cases version ID isn't represented in the original > Vendor-defined structure, like "1.01". The only part which could be > considered as better in DWC3 approach is having a macro name, which gives > a bit better notion about the operation. But does it really worth > introducing a new abstraction in the driver? > > On the other hand we could intermix the approaches. For instance decode > the Component version as I suggested in this patch and implement a set of > version checking macro. Thus we won't need so many additional macro > encoding the SSI_COMP_VERSION content. > > If only we could have a macro like DW_SPI_VER(dws, >=, "1.01a") with no > performance drawbacks I'd be glad to use it. AFAIU compiler can't > operate with the string literal symbols, thus the symbols extraction > like "1.01a"[0] will be performed on each statement execution which isn't > that performant comparing to a simple two integers comparison. > > BTW note the DWC3 macros implicitly depend on having a local variable > with dwc name which violates the kernel coding style. > > > > > > + } > > > > > > > Have you seen this, btw? > > > > > > > > https://elixir.bootlin.com/linux/latest/source/drivers/ > > > tty/serial/8250/8250_dwlib.c#L93 > > > > > > It doesn't utilized version ID for something functional, but just > > > prints it to the console. So it isn't that good reference in this > > > case. Depends what you would like to do with this. If it's only for information to the developer, then it fits, if you need to compare, then see above.
On Sat, Nov 13, 2021 at 03:15:41PM +0200, Andy Shevchenko wrote: > On Sat, Nov 13, 2021 at 2:19 AM Serge Semin <fancer.lancer@gmail.com> wrote: > > On Sat, Nov 13, 2021 at 01:14:30AM +0200, Andy Shevchenko wrote: > > > On Saturday, November 13, 2021, Serge Semin <fancer.lancer@gmail.com> wrote: > > > > On Fri, Nov 12, 2021 at 11:37:36PM +0200, Andy Shevchenko wrote: > > > > > On Fri, Nov 12, 2021 at 11:27 PM Andy Shevchenko > > > > > <andy.shevchenko@gmail.com> wrote: > > > > > > On Fri, Nov 12, 2021 at 10:52 PM Serge Semin > > > > > > <Sergey.Semin@baikalelectronics.ru> wrote: > > ... > > > > > > > > + /* > > > > > > > + * Retrieve the Synopsys component version if it hasn't been > > > > specified > > > > > > > + * by the platform. Note the CoreKit version ID is encoded > > > > as a 4bytes > > > > > > > + * ASCII string enclosed with '*' symbol. > > > > > > > + */ > > > > > > > + if (!dws->ver) { > > > > > > > + u32 comp; > > > > > > > + > > > > > > > + comp = dw_readl(dws, DW_SPI_VERSION); > > > > > > > + dws->ver = (DW_SPI_GET_BYTE(comp, 3) - '0') * 100; > > > > > > > + dws->ver += (DW_SPI_GET_BYTE(comp, 2) - '0') * 10; > > > > > > > + dws->ver += (DW_SPI_GET_BYTE(comp, 1) - '0'); > > > > > > > + > > > > > > > + dev_dbg(dev, "Synopsys DWC%sSSI v%u.%02u\n", > > > > > > > + (dws->caps & DW_SPI_CAP_DWC_HSSI) ? " " : " > > > > APB ", > > > > > > > + dws->ver / 100, dws->ver % 100); > > > > > > > > > > > > > > > > Oh lą lą, first you multiply then you divide in the same piece of code! > > > > > > What's wrong with fourcc (and thus keep it in ver filed as is) ? (Also > > > > > > we have %p4cc) > > > > > > > > Please note that's just a dev_DBG() print. So division has been used > > > > in there to check whether the conversion was correct. The whole idea > > > > behind the code above it was to retrieve the Component version as a > > > > single number so then it could be used by the driver code in a simple > > > > statement with a normal integer operation. For instance in case if we > > > > need to check whether DW SSI IP-core version is greater than 1.01 we'd > > > > have something like this: if (dws->ver > 101). Here 101 looks at least > > > > close to the original 1.01. How would the statement look with four > > > > chars? Of course we could add an another macro which would look like > > > > this: > > > > #define DW_SSI_VER(_maj, _mid, _min) \ > > > > ((_maj) << 24 | (_mid) << 16 | (_min) << 8 | '*') > > > > and use it with raw version ID, like this > > > > (dws->ver > DW_SSI_VER('1', '0', '1')). But IMO it doesn't look > > > > better if not worse. > > > > Alternatively we could split the version ID into two parts with > > > > major and minor numbers. But normally one doesn't make much sense > > > > without another so each statement would need to check both of them > > > > at once anyway. So I decided to stick with a simplest solution and > > > > combined them into a single storage. Have you got a better idea of > > > > how to implement this functionality? > > > > > Then check DWC3 driver which relies on IZp version a lot. > > > > I'm still not convinced that the DWC3 solution would be better in this case. > > (I had a similar approach in mind though.) Although it might be suitable > > here seeing we could take the IP generation into account in a single > > macro. But at the same time having macros defined for each version may > > eventually turn into a clumsy set of macros space as it happened in DWC3. > > > > I don't understand what do you see wrong in the suggested here > > solution except a math in the debug string? > > The transformation ruins the fourcc [1] representation. We know that > this is an ASCII. We know the position and meaning of each from 4 > characters. > > [1]: https://en.wikipedia.org/wiki/FourCC So that four-CC thing wasn't Synopsys invention after all, but a sort of a relatively common approach. Your point finally starts making sense to me. > > > Why would you prefer the > > DWC3 approach better than the one implemented in my patch? > > You asked what is _better_ implementation than yours. It doesn't mean > the DWC3 approach fully fits here, but > - SPI DW has the same issue as DWC3 solves, i.e. different version > lines for two or more IPs of the same kind (see DWC3, DWC31, DWC32) > - it doesn't ruins 4cc > > In case if we need to print it, I would rather see something in %p4cc > implementation than customized 100500 approaches spreaded over the > entire kernel. Yep, these are the main pros of the DWC3 approach. Just to note in fact AFAICS Synopsys doesn't utilize the denoted components versioning for all its IP-cores. At least DW (G|X-G|XL-G)?MACs define either a normal one-byte component version ID (for instance version 3.3 looks as 0x33) or two-bytes with pair - IP-core and version IDs. But that likely is an exception, while the most of DWCs are indeed identified by the ASCII tuple. In our case we don't have the IP-core version explicitly encoded in the Component version register, so it is determined by the platform-specific code. With a minor effort I'll be able to just convert the DW_SPI_CAP_DWC_HSSI capability into two macros with virtual IP-core ID thus we'd have a more-or-less coherent versioning API. In this case we can retain the ASCII Version ID. Settled then. I'll send v2 with this patch reworked as you suggest. -Sergey > > > I don't really see much benefits in it: > > if (dws->ver > 101) > > or > > if (DW_SPI_VER_AFTER(dws, 101)) > > In both cases version ID isn't represented in the original > > Vendor-defined structure, like "1.01". The only part which could be > > considered as better in DWC3 approach is having a macro name, which gives > > a bit better notion about the operation. But does it really worth > > introducing a new abstraction in the driver? > > > > On the other hand we could intermix the approaches. For instance decode > > the Component version as I suggested in this patch and implement a set of > > version checking macro. Thus we won't need so many additional macro > > encoding the SSI_COMP_VERSION content. > > > > If only we could have a macro like DW_SPI_VER(dws, >=, "1.01a") with no > > performance drawbacks I'd be glad to use it. AFAIU compiler can't > > operate with the string literal symbols, thus the symbols extraction > > like "1.01a"[0] will be performed on each statement execution which isn't > > that performant comparing to a simple two integers comparison. > > > > BTW note the DWC3 macros implicitly depend on having a local variable > > with dwc name which violates the kernel coding style. > > > > > > > > + } > > > > > > > > > Have you seen this, btw? > > > > > > > > > > https://elixir.bootlin.com/linux/latest/source/drivers/ > > > > tty/serial/8250/8250_dwlib.c#L93 > > > > > > > > It doesn't utilized version ID for something functional, but just > > > > prints it to the console. So it isn't that good reference in this > > > > case. > > Depends what you would like to do with this. If it's only for > information to the developer, then it fits, if you need to compare, > then see above. > > -- > With Best Regards, > Andy Shevchenko
diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c index b4cbcd38eaba..1766a29ca790 100644 --- a/drivers/spi/spi-dw-core.c +++ b/drivers/spi/spi-dw-core.c @@ -823,6 +823,24 @@ static void dw_spi_hw_init(struct device *dev, struct dw_spi *dws) { dw_spi_reset_chip(dws); + /* + * Retrieve the Synopsys component version if it hasn't been specified + * by the platform. Note the CoreKit version ID is encoded as a 4bytes + * ASCII string enclosed with '*' symbol. + */ + if (!dws->ver) { + u32 comp; + + comp = dw_readl(dws, DW_SPI_VERSION); + dws->ver = (DW_SPI_GET_BYTE(comp, 3) - '0') * 100; + dws->ver += (DW_SPI_GET_BYTE(comp, 2) - '0') * 10; + dws->ver += (DW_SPI_GET_BYTE(comp, 1) - '0'); + + dev_dbg(dev, "Synopsys DWC%sSSI v%u.%02u\n", + (dws->caps & DW_SPI_CAP_DWC_HSSI) ? " " : " APB ", + dws->ver / 100, dws->ver % 100); + } + /* * Try to detect the FIFO depth if not set by interface driver, * the depth could be from 2 to 256 from HW spec diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h index 634085eadad1..d06857d8d173 100644 --- a/drivers/spi/spi-dw.h +++ b/drivers/spi/spi-dw.h @@ -149,6 +149,7 @@ struct dw_spi { u32 max_mem_freq; /* max mem-ops bus freq */ u32 max_freq; /* max bus freq supported */ + u32 ver; /* Synopsys component version */ u32 caps; /* DW SPI capabilities */ u32 reg_io_width; /* DR I/O width in bytes */
Each Synopsys DWC SSI controller is equipped with a Synopsys Component version register, which encodes a version ID of an IP-core the controller has been synthesized from. That can be useful in future for the version-based conditional features implementation in the driver. Note the component version is encoded as an ASCII string so we need to convert it from the string to a normal unsigned integer to be easily used then in the driver statements. Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> --- drivers/spi/spi-dw-core.c | 18 ++++++++++++++++++ drivers/spi/spi-dw.h | 1 + 2 files changed, 19 insertions(+)