linux-kernel - Re: [PATCH 9/9] mfd: ab8500-core: Pass GPADC compatible string to MFD core

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130514113426.GB28781@gmail.com>
Date:	Tue, 14 May 2013 12:34:26 +0100
From:	Lee Jones <lee.jones@...aro.org>
To:	Linus Walleij <linus.walleij@...aro.org>
Cc:	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Arnd Bergmann <arnd@...db.de>,
	Linus WALLEIJ <linus.walleij@...ricsson.com>,
	Srinidhi KASAGAR <srinidhi.kasagar@...ricsson.com>,
	Samuel Ortiz <sameo@...ux.intel.com>
Subject: Re: [PATCH 9/9] mfd: ab8500-core: Pass GPADC compatible string to
 MFD core

> >> probing and does not have an .of_match_table defined in it's
> >> driver struct? I mean, what could possibly match that
> >> compatible string? The .name field will take care of naming
> >> the device does it not?
> >
> > You only need that stuff if you require _extra_ bindings. Things like
> > regulators and interrupt numbers are configured behind the scenes.
> 
> Sure, but isn't the idea with of_match_table that this should be
> used in drivers when probing from device tree?

We're not probing from Device Tree. We're registering from MFD.

> Surely it *works* with other schemes but I always thought about
> them as some kind of rough fallback hacks. I would be way more
> convenient with the patch if it added an of_match_table to
> the ADC driver as well, because then I understand what is
> going on.

They're not full-back hacks. All drivers obtain their interrupts this
way when using Device Tree.

> >> For non-emergency merging though:
> >> Acked-by: Linus Walleij <linus.walleij@...aro.org>
> >
> > If we don't put this into v3.10-rcX, then the GPADC driver will be
> > broken in v3.10 when booting with DT.
> 
> OK I think I understand why now...
> 
> >> But for this to make sense the AB8500 ADC driver needs
> >> to be augmented for DT probing and preferrably also moved
> >> to drivers/adc and made to utilize that subsystem.
> >
> > Not sure I understand the reasoning for this. It works well as a plain
> > MFD device.
> 
> The commit message says:
>  "Without it, the driver won't be able to interrogate the Device Tree or
>   locate suitable regulators and will most likely fail to probe."
> 
> That commit message makes it sound like the driver is
> interrogating (hm is that the right term...) the device tree, but
> now you say it isn't (as you say it is a plain MFD device).
> Of course the MFD core and drivers/of/platform.c is doing
> all sorts of magic based on the device tree, and that is
> what the patch fixes, right? Nothing to do with the driver
> really?

I guess you're kind of correct. Yes, it's not the actual driver that's
interrogating (yes it's the right term) the Device Tree, other
subsystems are doing it on the driver's behalf. But the driver's node
is being used, which is why we need the compatible string.

> I suspect the missing regulators due to how the OF core or
> MFD core uses the device tree is the actual problem
> solved by the patch?
> 
> Anyway I think I was after this:
> 
> Isn't the idea that all devices that get probed from a device tree
> shall have corresponding bindings documented under
> Documentation/devicetree/bindings/*?

It has an entry in  Documentation/devicetree/bindings/mfd/ab8500.txt.

> And this driver is definately in the wrong subsystem (not your
> fault) so I suspect the binding doc should be in
> Documentation/devicetree/bindings/adc/ab8500.txt or
> something.

All of the AB8500 devices are covered in the aforementioned file and
to my knowledge none of them are represented in their respective
'real' driver type's binding doc folders (except Power).

> I'm not pointing that out as a problem in this
> patch, I'm just discussing the mess we're in ...

Sure.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/