On Fri, Jun 13, 2014 at 08:45:07AM -0400, [email protected] wrote: > On Fri, Jun 13, 2014 at 8:17 AM, Maxime Ripard > <[email protected]> wrote: > > On Fri, Jun 13, 2014 at 08:09:36AM -0400, [email protected] wrote: > >> On Fri, Jun 13, 2014 at 6:15 AM, Maxime Ripard > >> <[email protected]> wrote: > >> > On Fri, Jun 13, 2014 at 11:54:06AM +0200, Hans de Goede wrote: > >> >> Hi, > >> >> > >> >> On 06/13/2014 10:40 AM, Maxime Ripard wrote: > >> >> > On Fri, Jun 13, 2014 at 09:32:20AM +0200, Hans de Goede wrote: > >> >> >> Hi, > >> >> >> > >> >> >> On 06/13/2014 12:48 AM, [email protected] wrote: > >> >> >>> On Thu, Jun 12, 2014 at 6:35 PM, Emilio López > >> >> >>> <[email protected]> wrote: > >> >> >>>> El 12/06/14 19:11, [email protected] escribió: > >> >> >>>> > >> >> >>>>> What has replaced sw_get_ic_ver() on 3.15? > >> >> >>>>> > >> >> >>>>> Codec code varies on every chip revision A,B,C and A10/20. > >> >> >>>> > >> >> >>>> > >> >> >>>> A10/A20 can be determined by the compatible string. Chip revision > >> >> >>>> is going > >> >> >>>> to be trickier though, there is no direct replacement of > >> >> >>>> sw_get_ic_ver() > >> >> >>>> that I'm aware of. sw_get_ic_ver() seems to poke a timer register > >> >> >>>> in the > >> >> >>>> sun4i case, and SID (for which we do have a driver[1]) on the > >> >> >>>> sun5i case. > >> >> >>>> > >> >> >>>> [1] drivers/misc/eeprom/sunxi_sid.c > >> >> >>> > >> >> >>> We may have to reimplement it. Codec driver has stuff like this in > >> >> >>> it. > >> >> >> > >> >> >> I think adding some sort of SoC version detection makes sense, so go > >> >> >> for it. > >> >> > > >> >> > It might, and we probably will come to it eventually, but I don't get > >> >> > what it would bring here. > >> >> > > >> >> > Have different compatible strings for the various revisions of the IP > >> >> > is much simpler and adds no code at all. > >> >> > >> >> That assumes that for a single board only a single revision of the SoC > >> >> has > >> >> ever been used. I would not be so sure that that is the case, I'm pretty > >> >> sure that there were various rruns of the original mk802 A10 version, > >> >> likely with the first runs having A10 Revision A and later runs > >> >> revision B. I really don't want to have to do different dts files just > >> >> to deal with this, that is not helpful from a maintenance pov, and it > >> >> will also only serve to confuse our end users as they will have no idea > >> >> which revision of the SoC they have, so solving the differences between > >> >> the A10 revision A vs B/C with a compatible string seems counter > >> >> productive. > >> > > >> > There's usually two patterns to deal with this: > >> > - Either have two different DT, depending on the revision of the > >> > board > >> > > >> > - If the board rev hasn't changed, have the machine code come and > >> > update the DT with the appropriate compatible (see > >> > > >> > http://lxr.free-electrons.com/source/arch/arm/mach-mvebu/board-v7.c#L71) > >> > >> This code fixes up the I2C compatible string based on SOC ID. That > >> doesn't seem right to me. Instead I would have put some code in early > >> boot that fixes up the root compatible string. The revision is a > >> property of the CPU. > >> > >> Change this: > >> compatible = "cubietech,cubietruck", "allwinner,sun7i-a20" > >> to: > >> compatible = "cubietech,cubietruck", "allwinner,sun7i-a20a", > >> "allwinner,sun7i-a20" > > > > While this would be true, you'd still have to change the sound codec > > compatible. And we're back to step 1. > > With this method I don't need to encode the knowledge that the sound > codec (or I2C) is broken into the machine file.
It's not broken, it behaves differently. Therefore, there not
compatible, and they should have different compatible for the *codec*
itself. It's just as simple as that.
> Instead you always modify the machine compatible string to add the
> revision. Then do this test over in the device driver.
How do you modify this? Like I said, u-boot is not an option.
> In device driver....
> if (of_machine_is_compatible("allwinner,sun7i-a20a")) {
> then do fix up
> }
>
> driver already has several if-thens for a10 vs a20.
I'm sorry, but the argument "the code is done that way" is definitely
not a good one.
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
signature.asc
Description: Digital signature
