linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* a case for a common efuse API?
@ 2014-07-08 20:00 Stephen Boyd
  2014-07-08 20:26 ` Olof Johansson
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Stephen Boyd @ 2014-07-08 20:00 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On MSM chips we have some efuses (called qfprom) where we store things
like calibration data, speed bins, etc. We need to read out data from
the efuses in various drivers like the cpufreq, thermal, etc. This
essentially boils down to a bunch of readls on the efuse from a handful
of different drivers. In devicetree this looks a little odd because
these drivers end up having an extra reg property (or two) that points
to a register in the efuse and some length, i.e you see this:

	thermal-sensor at 34000 {
		compatible = "sensor";
		reg = <0x34000 0x1000>, <0x10018 0xc>;
		reg-names = "sensor", "efuse_calib";
	}


I imagine in DT we want something more like this:

	efuse: efuse at 10000 {
		compatible = "efuse";
		reg = <0x10000 0x1000>;
	}

	thermal-sensor at 34000 {
		compatible = "sensor";
		reg = <0x34000 0x1000>;
		efuse = <&efuse 0x18>;
	}


Where we can point to the efuse and give an address offset. From code we
could then call some efuse_read() function to read the sensor's
calibration data.

It's been suggested that we use syscon for this, but I wonder if that is
the right thing to do. With a syscon you're usually writing some
registers that got lumped together with other registers that aren't
directly related to your driver. It doesn't seem like syscon is made for
reading fuses or other things like eeproms. Maybe I'm wrong though.

Using syscon would probably work if we could add a way to point to
offsets within the syscon node (the 0x18 part in the example). In my
specific use-case the calibration data may have different offsets
depending on which SoC the hardware is instantiated in so we could also
make syscon work if the compatible field for the sensor node indicates
which SoC it is.

I added Tegra folks because I see that on Tegra this hardware is exposed
via an SoC specific API, tegra_fuse_readl(), and an associated driver in
drivers/misc/fuse/tegra/. Unfortunately I don't see any users of the API
outside of the speedo code in the same directory and the sysfs bin
attribute that may or may not be in use by some userspace code.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 16+ messages in thread

* a case for a common efuse API?
  2014-07-08 20:00 a case for a common efuse API? Stephen Boyd
@ 2014-07-08 20:26 ` Olof Johansson
  2014-07-08 21:59 ` Bjorn Andersson
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Olof Johansson @ 2014-07-08 20:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 8, 2014 at 1:00 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:

> I added Tegra folks because I see that on Tegra this hardware is exposed
> via an SoC specific API, tegra_fuse_readl(), and an associated driver in
> drivers/misc/fuse/tegra/. Unfortunately I don't see any users of the API
> outside of the speedo code in the same directory and the sysfs bin
> attribute that may or may not be in use by some userspace code.

Note that I also turned down the pull request that houses the code
under drivers/misc.

This is the type of stuff we've been talking about needing a home for
somewhere, and drivers/soc was one of the ones mentioned for that. I
really don't like adding dependencies on a random driver from platform
code, for example.


-Olof

^ permalink raw reply	[flat|nested] 16+ messages in thread

* a case for a common efuse API?
  2014-07-08 20:00 a case for a common efuse API? Stephen Boyd
  2014-07-08 20:26 ` Olof Johansson
@ 2014-07-08 21:59 ` Bjorn Andersson
  2014-07-09  7:24 ` Uwe Kleine-König
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Bjorn Andersson @ 2014-07-08 21:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 8, 2014 at 1:00 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> Hi,
>
> On MSM chips we have some efuses (called qfprom) where we store things
> like calibration data, speed bins, etc. We need to read out data from
> the efuses in various drivers like the cpufreq, thermal, etc. This
> essentially boils down to a bunch of readls on the efuse from a handful
> of different drivers. In devicetree this looks a little odd because
> these drivers end up having an extra reg property (or two) that points
> to a register in the efuse and some length, i.e you see this:

Not only does it look odd, but it limits the possible use to devices on the
mmio bus.

[...]
> It's been suggested that we use syscon for this, but I wonder if that is
> the right thing to do. With a syscon you're usually writing some
> registers that got lumped together with other registers that aren't
> directly related to your driver. It doesn't seem like syscon is made for
> reading fuses or other things like eeproms. Maybe I'm wrong though.
>

I've been thinking further about this since we discussed it; and I agree with
you that this probably wasn't in the original plans for syscon.

But looking at the qfprom hardware, we have a span of 32 bit registers that's
supposed to be accessed from various drivers. All these registers are the same
size, so abstracting them with a mmio-regmap makes sense. All we need is a way
for drivers to get a handle to this regmap.

This was exactly what I implemented and then I realized that I had only created
a copy of syscon (although readonly) and given it a different name.


A potential addition to this would be to expose a (preferrably separate) api to
fuse things, for factory use. But looking at the qfprom this is done by writing
to different parts of the register space, so we could simply add a factory fuse
driver on the side - once one is desired.

> Using syscon would probably work if we could add a way to point to
> offsets within the syscon node (the 0x18 part in the example). In my
> specific use-case the calibration data may have different offsets
> depending on which SoC the hardware is instantiated in so we could also
> make syscon work if the compatible field for the sensor node indicates
> which SoC it is.
>

I'm not sure if other users of syscon would find a use for an xlate, if not
then we could just have that as a separate property that lists the reg offsets
within the syscon.

Maybe there's some benefits of common naming of said property so we could put
some common accessor functions somewhere?

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 16+ messages in thread

* a case for a common efuse API?
  2014-07-08 20:00 a case for a common efuse API? Stephen Boyd
  2014-07-08 20:26 ` Olof Johansson
  2014-07-08 21:59 ` Bjorn Andersson
@ 2014-07-09  7:24 ` Uwe Kleine-König
  2014-07-09  7:50 ` Srinivas Kandagatla
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Uwe Kleine-König @ 2014-07-09  7:24 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Tue, Jul 08, 2014 at 01:00:23PM -0700, Stephen Boyd wrote:
> On MSM chips we have some efuses (called qfprom) where we store things
> like calibration data, speed bins, etc.
I don't have something constructive to say here, just want to point out
that on efm32 there is something similar. It's called "Device
Information (DI) Page" and contains "calibration values, a
unique identification number and other useful data".

Currently it is unused in mainline, but in my tree I access it directly
to determine cpu type (part number, part family number and Production
ID), flash and ram size, and the calibration settings for the ADC.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 16+ messages in thread

* a case for a common efuse API?
  2014-07-08 20:00 a case for a common efuse API? Stephen Boyd
                   ` (2 preceding siblings ...)
  2014-07-09  7:24 ` Uwe Kleine-König
@ 2014-07-09  7:50 ` Srinivas Kandagatla
  2014-07-09  8:35 ` Maxime Ripard
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Srinivas Kandagatla @ 2014-07-09  7:50 UTC (permalink / raw)
  To: linux-arm-kernel



On 08/07/14 21:00, Stephen Boyd wrote:
> Hi,
>
> On MSM chips we have some efuses (called qfprom) where we store things
> like calibration data, speed bins, etc. We need to read out data from
> the efuses in various drivers like the cpufreq, thermal, etc. This
> essentially boils down to a bunch of readls on the efuse from a handful
> of different drivers. In devicetree this looks a little odd because
> these drivers end up having an extra reg property (or two) that points
> to a register in the efuse and some length, i.e you see this:
>
> 	thermal-sensor at 34000 {
> 		compatible = "sensor";
> 		reg = <0x34000 0x1000>, <0x10018 0xc>;
> 		reg-names = "sensor", "efuse_calib";
> 	}
>
>
> I imagine in DT we want something more like this:
>
> 	efuse: efuse at 10000 {
> 		compatible = "efuse";
> 		reg = <0x10000 0x1000>;
> 	}
>
> 	thermal-sensor at 34000 {
> 		compatible = "sensor";
> 		reg = <0x34000 0x1000>;
> 		efuse = <&efuse 0x18>;
> 	}
>
>
> Where we can point to the efuse and give an address offset. From code we
> could then call some efuse_read() function to read the sensor's
> calibration data.
>
> It's been suggested that we use syscon for this, but I wonder if that is
> the right thing to do. With a syscon you're usually writing some
> registers that got lumped together with other registers that aren't
> directly related to your driver. It doesn't seem like syscon is made for
> reading fuses or other things like eeproms. Maybe I'm wrong though.
>

I remember there was a similar discussion some time last year about 
this:  https://lkml.org/lkml/2013/7/5/361


> Using syscon would probably work if we could add a way to point to
> offsets within the syscon node (the 0x18 part in the example). In my
> specific use-case the calibration data may have different offsets
> depending on which SoC the hardware is instantiated in so we could also
> make syscon work if the compatible field for the sensor node indicates
> which SoC it is.

Something like this:

	efuse: efuse at 10000 {
  		compatible = "efuse", "syscon";
  		reg = <0x10000 0x1000>;
  	}

  	thermal-sensor at 34000 {
  		compatible = "sensor", "soc-xyz";
  		reg = <0x34000 0x1000>;
		syscon = <&efuse>;
  	}

sensor driver could add of_data for the soc-xyz which would contain soc 
specific efuse offsets.


--srini

>
> I added Tegra folks because I see that on Tegra this hardware is exposed
> via an SoC specific API, tegra_fuse_readl(), and an associated driver in
> drivers/misc/fuse/tegra/. Unfortunately I don't see any users of the API
> outside of the speedo code in the same directory and the sysfs bin
> attribute that may or may not be in use by some userspace code.
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* a case for a common efuse API?
  2014-07-08 20:00 a case for a common efuse API? Stephen Boyd
                   ` (3 preceding siblings ...)
  2014-07-09  7:50 ` Srinivas Kandagatla
@ 2014-07-09  8:35 ` Maxime Ripard
  2014-07-09 23:32   ` Stephen Boyd
  2014-07-09 11:49 ` Peter De Schrijver
  2014-07-09 20:42 ` Tomasz Figa
  6 siblings, 1 reply; 16+ messages in thread
From: Maxime Ripard @ 2014-07-09  8:35 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Stephen,

On Tue, Jul 08, 2014 at 01:00:23PM -0700, Stephen Boyd wrote:
> Hi,
> 
> On MSM chips we have some efuses (called qfprom) where we store things
> like calibration data, speed bins, etc. We need to read out data from
> the efuses in various drivers like the cpufreq, thermal, etc. This
> essentially boils down to a bunch of readls on the efuse from a handful
> of different drivers. In devicetree this looks a little odd because
> these drivers end up having an extra reg property (or two) that points
> to a register in the efuse and some length, i.e you see this:
> 
> 	thermal-sensor at 34000 {
> 		compatible = "sensor";
> 		reg = <0x34000 0x1000>, <0x10018 0xc>;
> 		reg-names = "sensor", "efuse_calib";
> 	}
> 
> 
> I imagine in DT we want something more like this:
> 
> 	efuse: efuse at 10000 {
> 		compatible = "efuse";
> 		reg = <0x10000 0x1000>;
> 	}
> 
> 	thermal-sensor at 34000 {
> 		compatible = "sensor";
> 		reg = <0x34000 0x1000>;
> 		efuse = <&efuse 0x18>;
> 	}

We have pretty much the same things in the Allwinner SoCs. We have an
efuse directly mapped into memory, with a few informations like a MAC
address, the SoC ID, the serial number, some RSA keys for the device,
etc.

The thing is, some boards expose these informations in an external
EEPROM as well.

I started working and went quite far to create an "eeprom" framework
to handle these cases, with a dt representation similar to what you
were exposing.

https://github.com/mripard/linux/tree/eeprom-framework-at24

It was working quite well, I was about to send it, but was told that I
should all be moved to MTD, and given up on it.

Anyway, +1 for this :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140709/87e3b136/attachment.sig>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* a case for a common efuse API?
  2014-07-08 20:00 a case for a common efuse API? Stephen Boyd
                   ` (4 preceding siblings ...)
  2014-07-09  8:35 ` Maxime Ripard
@ 2014-07-09 11:49 ` Peter De Schrijver
  2014-07-21 15:51   ` Stephen Warren
  2014-07-09 20:42 ` Tomasz Figa
  6 siblings, 1 reply; 16+ messages in thread
From: Peter De Schrijver @ 2014-07-09 11:49 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 08, 2014 at 10:00:23PM +0200, Stephen Boyd wrote:
> 
> I added Tegra folks because I see that on Tegra this hardware is exposed
> via an SoC specific API, tegra_fuse_readl(), and an associated driver in
> drivers/misc/fuse/tegra/. Unfortunately I don't see any users of the API
> outside of the speedo code in the same directory and the sysfs bin
> attribute that may or may not be in use by some userspace code.
> 

The SATA driver by Mikko Perttunen uses it. The driver hasn't been merged
though. There's some more upcoming work which makes use of it.
I don't think we can standardize much of an API for this. The data is
SoC specific, so the user will always need to have some SoC specific
knowledge on how to use it. In some cases we need it very early (eg. to
determine the correct Tegra20 revision). On Tegra20 we can't use the CPU
to read the fuses due to a hw bug, we have to use DMA, which means the
transaction becomes blocking and could fail due to lack of resources.

Cheers,

Peter.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* a case for a common efuse API?
  2014-07-08 20:00 a case for a common efuse API? Stephen Boyd
                   ` (5 preceding siblings ...)
  2014-07-09 11:49 ` Peter De Schrijver
@ 2014-07-09 20:42 ` Tomasz Figa
  6 siblings, 0 replies; 16+ messages in thread
From: Tomasz Figa @ 2014-07-09 20:42 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Stephen,

On 08.07.2014 22:00, Stephen Boyd wrote:
> Hi,
> 
> On MSM chips we have some efuses (called qfprom) where we store things
> like calibration data, speed bins, etc. We need to read out data from
> the efuses in various drivers like the cpufreq, thermal, etc. This
> essentially boils down to a bunch of readls on the efuse from a handful
> of different drivers.

We have similar thing on Exynos SoCs as well. If you're interested in
looking at our use cases, then there was an RFC series posted quite a
long time ago [1], but then things stalled.

[1] https://lkml.org/lkml/2013/11/15/155

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 16+ messages in thread

* a case for a common efuse API?
  2014-07-09  8:35 ` Maxime Ripard
@ 2014-07-09 23:32   ` Stephen Boyd
  2014-07-10 14:26     ` Maxime Ripard
  0 siblings, 1 reply; 16+ messages in thread
From: Stephen Boyd @ 2014-07-09 23:32 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/09/14 01:35, Maxime Ripard wrote:
> Hi Stephen,
>
> On Tue, Jul 08, 2014 at 01:00:23PM -0700, Stephen Boyd wrote:
>> Hi,
>>
>> On MSM chips we have some efuses (called qfprom) where we store things
>> like calibration data, speed bins, etc. We need to read out data from
>> the efuses in various drivers like the cpufreq, thermal, etc. This
>> essentially boils down to a bunch of readls on the efuse from a handful
>> of different drivers. In devicetree this looks a little odd because
>> these drivers end up having an extra reg property (or two) that points
>> to a register in the efuse and some length, i.e you see this:
>>
>> 	thermal-sensor at 34000 {
>> 		compatible = "sensor";
>> 		reg = <0x34000 0x1000>, <0x10018 0xc>;
>> 		reg-names = "sensor", "efuse_calib";
>> 	}
>>
>>
>> I imagine in DT we want something more like this:
>>
>> 	efuse: efuse at 10000 {
>> 		compatible = "efuse";
>> 		reg = <0x10000 0x1000>;
>> 	}
>>
>> 	thermal-sensor at 34000 {
>> 		compatible = "sensor";
>> 		reg = <0x34000 0x1000>;
>> 		efuse = <&efuse 0x18>;
>> 	}
> We have pretty much the same things in the Allwinner SoCs. We have an
> efuse directly mapped into memory, with a few informations like a MAC
> address, the SoC ID, the serial number, some RSA keys for the device,
> etc.
>
> The thing is, some boards expose these informations in an external
> EEPROM as well.
>
> I started working and went quite far to create an "eeprom" framework
> to handle these cases, with a dt representation similar to what you
> were exposing.
>
> https://github.com/mripard/linux/tree/eeprom-framework-at24
>
> It was working quite well, I was about to send it, but was told that I
> should all be moved to MTD, and given up on it.

Did anything ever get merged? Or the whole thing was dropped?

That branch looks like what I want, assuming we could get an agreement
on the binding. It looks like pretty much every SoC has this, and there
isn't any API or binding for it that I've seen. The only thing I see is
Documentation/devicetree/bindings/eeprom.txt and that doesn't cover the
client aspect at all.

Taking a quick peek at the code, it might be better to change the read
API to take a buffer and length, so that the caller doesn't need to free
the data allocated by the eeprom layer. It also makes it symmetrical
with the write API. We'd probably also need to make it work really early
for SoC's like Tegra where we want to read the SoC revision early. So
probably split off the device registration part to a later time to allow
register() to be called early.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 16+ messages in thread

* a case for a common efuse API?
  2014-07-09 23:32   ` Stephen Boyd
@ 2014-07-10 14:26     ` Maxime Ripard
  2014-07-10 15:18       ` Alexandre Belloni
                         ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Maxime Ripard @ 2014-07-10 14:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jul 09, 2014 at 04:32:03PM -0700, Stephen Boyd wrote:
> On 07/09/14 01:35, Maxime Ripard wrote:
> > Hi Stephen,
> >
> > On Tue, Jul 08, 2014 at 01:00:23PM -0700, Stephen Boyd wrote:
> >> Hi,
> >>
> >> On MSM chips we have some efuses (called qfprom) where we store things
> >> like calibration data, speed bins, etc. We need to read out data from
> >> the efuses in various drivers like the cpufreq, thermal, etc. This
> >> essentially boils down to a bunch of readls on the efuse from a handful
> >> of different drivers. In devicetree this looks a little odd because
> >> these drivers end up having an extra reg property (or two) that points
> >> to a register in the efuse and some length, i.e you see this:
> >>
> >> 	thermal-sensor at 34000 {
> >> 		compatible = "sensor";
> >> 		reg = <0x34000 0x1000>, <0x10018 0xc>;
> >> 		reg-names = "sensor", "efuse_calib";
> >> 	}
> >>
> >>
> >> I imagine in DT we want something more like this:
> >>
> >> 	efuse: efuse at 10000 {
> >> 		compatible = "efuse";
> >> 		reg = <0x10000 0x1000>;
> >> 	}
> >>
> >> 	thermal-sensor at 34000 {
> >> 		compatible = "sensor";
> >> 		reg = <0x34000 0x1000>;
> >> 		efuse = <&efuse 0x18>;
> >> 	}
> > We have pretty much the same things in the Allwinner SoCs. We have an
> > efuse directly mapped into memory, with a few informations like a MAC
> > address, the SoC ID, the serial number, some RSA keys for the device,
> > etc.
> >
> > The thing is, some boards expose these informations in an external
> > EEPROM as well.
> >
> > I started working and went quite far to create an "eeprom" framework
> > to handle these cases, with a dt representation similar to what you
> > were exposing.
> >
> > https://github.com/mripard/linux/tree/eeprom-framework-at24
> >
> > It was working quite well, I was about to send it, but was told that I
> > should all be moved to MTD, and given up on it.
> 
> Did anything ever get merged? Or the whole thing was dropped?

Nope, I just never posted it. I could send it as an RFC though, and
see what are the feedbacks.

> That branch looks like what I want, assuming we could get an agreement
> on the binding. It looks like pretty much every SoC has this, and there
> isn't any API or binding for it that I've seen. The only thing I see is
> Documentation/devicetree/bindings/eeprom.txt and that doesn't cover the
> client aspect at all.
> 
> Taking a quick peek at the code, it might be better to change the read
> API to take a buffer and length, so that the caller doesn't need to free
> the data allocated by the eeprom layer. It also makes it symmetrical
> with the write API. We'd probably also need to make it work really early
> for SoC's like Tegra where we want to read the SoC revision early. So
> probably split off the device registration part to a later time to allow
> register() to be called early.

I guess that the kind of things we could discuss after posting these
patches, but yep, it looks reasonnable.

I'll try to get things a bit cleaner, and post them in the next days.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140710/7e3b54ab/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* a case for a common efuse API?
  2014-07-10 15:41       ` Grygorii Strashko
@ 2014-07-10 15:09         ` Maxime Ripard
  0 siblings, 0 replies; 16+ messages in thread
From: Maxime Ripard @ 2014-07-10 15:09 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jul 10, 2014 at 06:41:55PM +0300, Grygorii Strashko wrote:
> On 07/10/2014 05:26 PM, Maxime Ripard wrote:
> >On Wed, Jul 09, 2014 at 04:32:03PM -0700, Stephen Boyd wrote:
> >>On 07/09/14 01:35, Maxime Ripard wrote:
> >>>Hi Stephen,
> >>>
> >>>On Tue, Jul 08, 2014 at 01:00:23PM -0700, Stephen Boyd wrote:
> >>>>Hi,
> >>>>
> >>>>On MSM chips we have some efuses (called qfprom) where we store things
> >>>>like calibration data, speed bins, etc. We need to read out data from
> >>>>the efuses in various drivers like the cpufreq, thermal, etc. This
> >>>>essentially boils down to a bunch of readls on the efuse from a handful
> >>>>of different drivers. In devicetree this looks a little odd because
> >>>>these drivers end up having an extra reg property (or two) that points
> >>>>to a register in the efuse and some length, i.e you see this:
> >>>>
> >>>>	thermal-sensor at 34000 {
> >>>>		compatible = "sensor";
> >>>>		reg = <0x34000 0x1000>, <0x10018 0xc>;
> >>>>		reg-names = "sensor", "efuse_calib";
> >>>>	}
> >>>>
> >>>>
> >>>>I imagine in DT we want something more like this:
> >>>>
> >>>>	efuse: efuse at 10000 {
> >>>>		compatible = "efuse";
> >>>>		reg = <0x10000 0x1000>;
> >>>>	}
> >>>>
> >>>>	thermal-sensor at 34000 {
> >>>>		compatible = "sensor";
> >>>>		reg = <0x34000 0x1000>;
> >>>>		efuse = <&efuse 0x18>;
> >>>>	}
> 
> Why don't use "syscon" framework for your needs? (mfd/syscon.c)

Because syscon is restricted to MMIO, and not really meant to be used
for this.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140710/4d5a6c38/attachment.sig>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* a case for a common efuse API?
  2014-07-10 14:26     ` Maxime Ripard
@ 2014-07-10 15:18       ` Alexandre Belloni
  2014-07-10 15:41       ` Grygorii Strashko
  2014-09-11 21:59       ` Stephen Boyd
  2 siblings, 0 replies; 16+ messages in thread
From: Alexandre Belloni @ 2014-07-10 15:18 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 10/07/2014 at 16:26:16 +0200, Maxime Ripard wrote :
> On Wed, Jul 09, 2014 at 04:32:03PM -0700, Stephen Boyd wrote:
> > On 07/09/14 01:35, Maxime Ripard wrote:
> > > Hi Stephen,
> > >
> > > On Tue, Jul 08, 2014 at 01:00:23PM -0700, Stephen Boyd wrote:
> > >> Hi,
> > >>
> > >> On MSM chips we have some efuses (called qfprom) where we store things
> > >> like calibration data, speed bins, etc. We need to read out data from
> > >> the efuses in various drivers like the cpufreq, thermal, etc. This
> > >> essentially boils down to a bunch of readls on the efuse from a handful
> > >> of different drivers. In devicetree this looks a little odd because
> > >> these drivers end up having an extra reg property (or two) that points
> > >> to a register in the efuse and some length, i.e you see this:
> > >>
> > >> 	thermal-sensor at 34000 {
> > >> 		compatible = "sensor";
> > >> 		reg = <0x34000 0x1000>, <0x10018 0xc>;
> > >> 		reg-names = "sensor", "efuse_calib";
> > >> 	}
> > >>
> > >>
> > >> I imagine in DT we want something more like this:
> > >>
> > >> 	efuse: efuse at 10000 {
> > >> 		compatible = "efuse";
> > >> 		reg = <0x10000 0x1000>;
> > >> 	}
> > >>
> > >> 	thermal-sensor at 34000 {
> > >> 		compatible = "sensor";
> > >> 		reg = <0x34000 0x1000>;
> > >> 		efuse = <&efuse 0x18>;
> > >> 	}
> > > We have pretty much the same things in the Allwinner SoCs. We have an
> > > efuse directly mapped into memory, with a few informations like a MAC
> > > address, the SoC ID, the serial number, some RSA keys for the device,
> > > etc.
> > >
> > > The thing is, some boards expose these informations in an external
> > > EEPROM as well.
> > >
> > > I started working and went quite far to create an "eeprom" framework
> > > to handle these cases, with a dt representation similar to what you
> > > were exposing.
> > >
> > > https://github.com/mripard/linux/tree/eeprom-framework-at24
> > >
> > > It was working quite well, I was about to send it, but was told that I
> > > should all be moved to MTD, and given up on it.
> > 
> > Did anything ever get merged? Or the whole thing was dropped?
> 
> Nope, I just never posted it. I could send it as an RFC though, and
> see what are the feedbacks.
> 
> > That branch looks like what I want, assuming we could get an agreement
> > on the binding. It looks like pretty much every SoC has this, and there
> > isn't any API or binding for it that I've seen. The only thing I see is
> > Documentation/devicetree/bindings/eeprom.txt and that doesn't cover the
> > client aspect at all.
> > 
> > Taking a quick peek at the code, it might be better to change the read
> > API to take a buffer and length, so that the caller doesn't need to free
> > the data allocated by the eeprom layer. It also makes it symmetrical
> > with the write API. We'd probably also need to make it work really early
> > for SoC's like Tegra where we want to read the SoC revision early. So
> > probably split off the device registration part to a later time to allow
> > register() to be called early.
> 
> I guess that the kind of things we could discuss after posting these
> patches, but yep, it looks reasonnable.
> 
> I'll try to get things a bit cleaner, and post them in the next days.
> 

Be aware that some SoCs are storing their OPPs there so this would be
useful if that framework is available early enough to register those to
cpufreq.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* a case for a common efuse API?
  2014-07-10 14:26     ` Maxime Ripard
  2014-07-10 15:18       ` Alexandre Belloni
@ 2014-07-10 15:41       ` Grygorii Strashko
  2014-07-10 15:09         ` Maxime Ripard
  2014-09-11 21:59       ` Stephen Boyd
  2 siblings, 1 reply; 16+ messages in thread
From: Grygorii Strashko @ 2014-07-10 15:41 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/10/2014 05:26 PM, Maxime Ripard wrote:
> On Wed, Jul 09, 2014 at 04:32:03PM -0700, Stephen Boyd wrote:
>> On 07/09/14 01:35, Maxime Ripard wrote:
>>> Hi Stephen,
>>>
>>> On Tue, Jul 08, 2014 at 01:00:23PM -0700, Stephen Boyd wrote:
>>>> Hi,
>>>>
>>>> On MSM chips we have some efuses (called qfprom) where we store things
>>>> like calibration data, speed bins, etc. We need to read out data from
>>>> the efuses in various drivers like the cpufreq, thermal, etc. This
>>>> essentially boils down to a bunch of readls on the efuse from a handful
>>>> of different drivers. In devicetree this looks a little odd because
>>>> these drivers end up having an extra reg property (or two) that points
>>>> to a register in the efuse and some length, i.e you see this:
>>>>
>>>> 	thermal-sensor at 34000 {
>>>> 		compatible = "sensor";
>>>> 		reg = <0x34000 0x1000>, <0x10018 0xc>;
>>>> 		reg-names = "sensor", "efuse_calib";
>>>> 	}
>>>>
>>>>
>>>> I imagine in DT we want something more like this:
>>>>
>>>> 	efuse: efuse at 10000 {
>>>> 		compatible = "efuse";
>>>> 		reg = <0x10000 0x1000>;
>>>> 	}
>>>>
>>>> 	thermal-sensor at 34000 {
>>>> 		compatible = "sensor";
>>>> 		reg = <0x34000 0x1000>;
>>>> 		efuse = <&efuse 0x18>;
>>>> 	}

Why don't use "syscon" framework for your needs? (mfd/syscon.c)

>>> We have pretty much the same things in the Allwinner SoCs. We have an
>>> efuse directly mapped into memory, with a few informations like a MAC
>>> address, the SoC ID, the serial number, some RSA keys for the device,
>>> etc.
>>>
>>> The thing is, some boards expose these informations in an external
>>> EEPROM as well.
>>>
>>> I started working and went quite far to create an "eeprom" framework
>>> to handle these cases, with a dt representation similar to what you
>>> were exposing.
>>>
>>> https://github.com/mripard/linux/tree/eeprom-framework-at24
>>>
>>> It was working quite well, I was about to send it, but was told that I
>>> should all be moved to MTD, and given up on it.
>>
>> Did anything ever get merged? Or the whole thing was dropped?
>
> Nope, I just never posted it. I could send it as an RFC though, and
> see what are the feedbacks.
>
>> That branch looks like what I want, assuming we could get an agreement
>> on the binding. It looks like pretty much every SoC has this, and there
>> isn't any API or binding for it that I've seen. The only thing I see is
>> Documentation/devicetree/bindings/eeprom.txt and that doesn't cover the
>> client aspect at all.
>>
>> Taking a quick peek at the code, it might be better to change the read
>> API to take a buffer and length, so that the caller doesn't need to free
>> the data allocated by the eeprom layer. It also makes it symmetrical
>> with the write API. We'd probably also need to make it work really early
>> for SoC's like Tegra where we want to read the SoC revision early. So
>> probably split off the device registration part to a later time to allow
>> register() to be called early.
>
> I guess that the kind of things we could discuss after posting these
> patches, but yep, it looks reasonnable.
>
> I'll try to get things a bit cleaner, and post them in the next days.
>

Regards,
-grygorii

^ permalink raw reply	[flat|nested] 16+ messages in thread

* a case for a common efuse API?
  2014-07-09 11:49 ` Peter De Schrijver
@ 2014-07-21 15:51   ` Stephen Warren
  0 siblings, 0 replies; 16+ messages in thread
From: Stephen Warren @ 2014-07-21 15:51 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/09/2014 05:49 AM, Peter De Schrijver wrote:
> On Tue, Jul 08, 2014 at 10:00:23PM +0200, Stephen Boyd wrote:
>>
>> I added Tegra folks because I see that on Tegra this hardware is exposed
>> via an SoC specific API, tegra_fuse_readl(), and an associated driver in
>> drivers/misc/fuse/tegra/. Unfortunately I don't see any users of the API
>> outside of the speedo code in the same directory and the sysfs bin
>> attribute that may or may not be in use by some userspace code.
>>
> 
> The SATA driver by Mikko Perttunen uses it. The driver hasn't been merged
> though. There's some more upcoming work which makes use of it.
> I don't think we can standardize much of an API for this. The data is
> SoC specific, so the user will always need to have some SoC specific
> knowledge on how to use it.

I think we could have a generic "read fuse X" API. The only thing that'd
be chip-specific is the value of "X". That could be hard-coded into the
client drivers, or perhaps represented in a DT propery e.g.
#fuse-cells/xxx-fuses. That said, I wonder what benefit we'd get from
such a common API. I suppose if any IP block was to be re-used in
completely different SoC families, it'd isolate the driver from having
to call different functions to read fuses on different SoC families, but
that doesn't seem to happen in practice yet, and the issue could be
solved later if needed.

It'd certainly be hard to create any higher-layer semantic API here,
since different SoCs store completely different sets of data in fuses,
so there would be little point in a common "read the MAC address from
the fuses" API, since that data wouldn't exist in fuses on many SoCs.

> In some cases we need it very early (eg. to
> determine the correct Tegra20 revision). On Tegra20 we can't use the CPU
> to read the fuses due to a hw bug, we have to use DMA, which means the
> transaction becomes blocking and could fail due to lack of resources.

Yes, any such API would become "least common-denominator" or similar;
i.e. any restriction imposed by any SoC would then affect how the API
works on any other SoC.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* a case for a common efuse API?
  2014-07-10 14:26     ` Maxime Ripard
  2014-07-10 15:18       ` Alexandre Belloni
  2014-07-10 15:41       ` Grygorii Strashko
@ 2014-09-11 21:59       ` Stephen Boyd
  2014-09-16 10:16         ` Maxime Ripard
  2 siblings, 1 reply; 16+ messages in thread
From: Stephen Boyd @ 2014-09-11 21:59 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/10/14 07:26, Maxime Ripard wrote:
>
> I guess that the kind of things we could discuss after posting these
> patches, but yep, it looks reasonnable.
>
> I'll try to get things a bit cleaner, and post them in the next days.
>

I never saw anything. Did you do any cleaning/posting? I'm going to try
it out today/tomorrow to see if it can make some things cleaner.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 16+ messages in thread

* a case for a common efuse API?
  2014-09-11 21:59       ` Stephen Boyd
@ 2014-09-16 10:16         ` Maxime Ripard
  0 siblings, 0 replies; 16+ messages in thread
From: Maxime Ripard @ 2014-09-16 10:16 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Stephen,

On Thu, Sep 11, 2014 at 02:59:08PM -0700, Stephen Boyd wrote:
> On 07/10/14 07:26, Maxime Ripard wrote:
> >
> > I guess that the kind of things we could discuss after posting these
> > patches, but yep, it looks reasonnable.
> >
> > I'll try to get things a bit cleaner, and post them in the next days.
> >
> 
> I never saw anything. Did you do any cleaning/posting? I'm going to try
> it out today/tomorrow to see if it can make some things cleaner.

Sorry, I never got any time to put things in shape besides rebasing.

Feel free to pick up the work.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140916/56ac37e4/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-09-16 10:16 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-08 20:00 a case for a common efuse API? Stephen Boyd
2014-07-08 20:26 ` Olof Johansson
2014-07-08 21:59 ` Bjorn Andersson
2014-07-09  7:24 ` Uwe Kleine-König
2014-07-09  7:50 ` Srinivas Kandagatla
2014-07-09  8:35 ` Maxime Ripard
2014-07-09 23:32   ` Stephen Boyd
2014-07-10 14:26     ` Maxime Ripard
2014-07-10 15:18       ` Alexandre Belloni
2014-07-10 15:41       ` Grygorii Strashko
2014-07-10 15:09         ` Maxime Ripard
2014-09-11 21:59       ` Stephen Boyd
2014-09-16 10:16         ` Maxime Ripard
2014-07-09 11:49 ` Peter De Schrijver
2014-07-21 15:51   ` Stephen Warren
2014-07-09 20:42 ` Tomasz Figa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).