Hi Dirk, On Wed, Jun 1, 2016 at 9:19 AM, Dirk Behme <dirk.behme@xxxxxxxxxxxx> wrote: > On 01.06.2016 07:19, Magnus Damm wrote: >> On Fri, May 27, 2016 at 4:56 PM, Dirk Behme <dirk.behme@xxxxxxxxxxxx> >> wrote: >>> On 27.05.2016 05:13, Magnus Damm wrote: >>>> I don't think anyone disagrees that it makes sense to be able to >>>> determine ES version during runtime. The questions in my mind are how >>>> to do it >>> >>> I've made a proposal ;) And I'm happy to discuss technically about it. >> >> Thanks! It would be interesting to know the reason behind your >> interest in these things. For instance, if you are interested in >> reducing run time memory usage, or general source code duplication >> reduction. Do you have any target platform that you can upstream board >> support for? >> >>>> and the urgency. >>> >>> Regarding the urgency: Someone has accepted the hard coded PRODUCT >>> register >>> (and MODEMR) being in renesas-drivers, now: >>> >>> https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/tree/drivers/gpu/drm/rcar-du/rcar_du_crtc.c?h=topic/gen3-latest#n177 >>> >>> This does really hurts us. >>> >>> Therefore, I try to get this removed as soon as possible to hopefully get >>> a >>> Renesas BSP without this, the next time. >>> >>> If anybody finds an other way to remove that, that would be fine, too. >> >> Please share the underlying issue so we can fix that. >> >>>> So far we have decided to use the compatible string since it is a >>>> common driver model abstraction already used by device drivers and >>>> hardware description in DT. Using DT compat string matching we can >>>> have logic in the drivers to handle ES differences if needed. As for >>>> how to enable the workaround, my opinion is that the missing piece >>>> consists of ES workaround code that appends ES suffix information to >>>> the DT compat strings automatically during boot. >>> >>> Technically this sounds slightly too complicated to me. As I have to >>> advocate my proposal (inspired from an other mainline SoC family) I'd >>> think >>> that my proposal is easier. >> >> It may indeed be easier in the short term, but I feel we need to >> consider how to support a wide range of SoCs in a consistent way. >> >>>> I suppose the workaround is not yet implemented because no one has >>>> deemed this topic as particularly urgent. Until it becomes urgent or a >>>> new ES version appears the affected users can simply adjust the DT >>>> binding themselves. This is what happened to the "sata-r8a7790-es1" >>>> case above. >> >> Can you see any technical reason why using DT compat strings wouldn't >> solve your case? > > I'll try to explain my understanding. Please re-ask or correct me if I fail > ;) > > First of all, I can talk only about R-Car3. There, at the moment, we have > ES1, ES2 and ES3 documented, already. We don't know, yet, if these are > really needed in the code, but it might be. > > Taking the example above, we then would have > > compatible = "renesas,sata-r8a7790", "sata-r8a7790-es1", "sata-r8a7790-es2", > "sata-r8a7790-es3"; > > in the device tree. Do we want that? No! Because its no run time detection. You would only have "renesas,sata-r8a7790" for normal parts. ES1 needs "renesas,sata-r8a7790-es1". "esX" compatible values are the exception, not the rule. > If we don't want that, and we want runtime detection, I think anywhere in > this thread it was mentioned that the "es1" string is appended to > sata-r8a7790 at runtime? If so, could you point us to the code for that? The PoC is on my local SSD. Will send out soon. > Then we have anything like > > if(PRODUCT_REG & ES1 == ES1) > append ES1 to the compatible string > > in the boot code. This is one additional indirection to my proposal, needing > boot time. In the driver you then add > > { > .compatible = "renesas,sata-r8a7790-es1", > .data = (void *)RCAR_R8A7790_ES1_SATA > }, > { > .compatible = "renesas,sata-r8a7790-es2", > .data = (void *)RCAR_R8A7790_ES2_SATA > }, > { > .compatible = "renesas,sata-r8a7790-es3", > .data = (void *)RCAR_R8A7790_ES3_SATA > }, No, only "renesas,sata-r8a7790" and "renesas,sata-r8a7790-es1", like we already have. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds