On Fri, Jun 13, 2014 at 9:58 AM, Maxime Ripard
<maxime.ripard@xxxxxxxxxxxxxxxxxx> wrote:
> On Fri, Jun 13, 2014 at 09:34:05AM -0400, jonsmirl@xxxxxxxxx wrote:
>> On Fri, Jun 13, 2014 at 9:20 AM, Maxime Ripard
>> <maxime.ripard@xxxxxxxxxxxxxxxxxx> wrote:
>> > On Fri, Jun 13, 2014 at 09:14:45AM -0400, jonsmirl@xxxxxxxxx wrote:
>> >> On Fri, Jun 13, 2014 at 9:08 AM, Arnd Bergmann <arnd@xxxxxxxx> wrote:
>> >> > On Friday 13 June 2014 09:05:14 jonsmirl@xxxxxxxxx wrote:
>> >> >> I was adding the CPU revision ID to the top level compatible string,
>> >> >> nothing specific to the device. By knowing the CPU revision I know
>> >> >> which errata to apply.
>> >> >>
>> >> >> If you patch the device strings, then you have to maintain knowledge
>> >> >> of which devices are broken in two places -- the device driver and the
>> >> >> machine file where you are patching the strings.
>> >> >
>> >> > IMHO, the driver only has to know what device versions exist, it
>> >> > shouldn't need to know which soc has which version of the device.
>> >> >
>> >> > Can you describe which kind of quirk you are looking at in the driver,
>> >> > and how common it is?
>> >>
>> >> Allwinner wired the volume control for their on-chip codec up
>> >> backwards on the A revsion of the the A10 chip. In later revisions is
>> >> it fixed to work in the right direction.
>> >>
>> >> I was wanting to add code like this to the device driver...
>> >>
>> >> if (of_machine_is_compatible("allwinner,sun7i-a20a")) {
>> >> fix bug
>> >> }
>> >>
>> >> Another solution would be for me to detect the chip revision in the
>> >> init code of the driver and remember it.
>> >
>> > The point is that you don't have that kind of constructs. You can
>> > associate any number of compatibles to your driver, and data to each
>> > of these compatibles. So, with different compatibles, you will be
>> > probed and retrieved these data directly, without even having this in
>> > your driver.
>>
>>
>> Then I end up with code in the machine file like this...
>>
>> find SOC revision
>> if revision == A
>> fix up codec and i2s driver compatible
>> if revision == B
>> fix up HDMI compatible
>> if revision == C
>> fix up whatever
>> I've seen chips with revision levels of J
>>
>> Why do you want this knowledge in the machine file? Why not just add
>> the test for SOC revision inside the device driver?
>
> Because it's going to be a mess anyway. No matter if it's in the
> driver or the machine code. So at least, that way, we're having it
> centralized and isolated from the driver's code.
The mess is always going to be in the driver code. You are just
changing the the string the driver code selects on...
if (of_machine_is_compatible("allwinner,sun7i-codec-rev-a")) {
fix bug
}
if (of_machine_is_compatible("allwinner,sun7i-a20a")) {
fix bug
}
The mess is not required in the machine file. Init time code can
always modify the machine compatible string like this with no side
effects. It would add 'a'-'j' as appropriate for the CPU.
Code changes this:
compatible = "cubietech,cubietruck", "allwinner,sun7i-a20"
to:
compatible = "cubietech,cubietruck", "allwinner,sun7i-a20-a",
"allwinner,sun7i-a20"
There is an existing API in the FDT code for changing compatible strings.
>
> Plus, we're going to need these several compatibles anyway, for boards
> that come with a single SoC revision, so why not just use them.
>
> And if it's a single revision of a single SoC for the moment, it's
> completely fine. When we'll get overwhelmed, if we ever are, we can
> always refactor that code to something smarter.
>
>> I do see the logic for wanting to change the compatible string for the
>> device, but these aren't really different devices. They are errata
>> being applied to the same device.
>
> No, they're not really different, but they're not the same either, or
> at least, for whatever reason, they don't behave in the exact same
> way.
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
--
Jon Smirl
jonsmirl@xxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
