Re: [RFC PATCH 1/3] eeprom: Add a simple EEPROM framework
From: Srinivas Kandagatla
Date: Tue Feb 24 2015 - 02:08:51 EST
Thanks Stephen for the comments.
On 23/02/15 23:11, Stephen Boyd wrote:
On 02/22/15 16:57, Rob Herring wrote:
On Sun, Feb 22, 2015 at 8:32 AM, Maxime Ripard
<maxime.ripard@xxxxxxxxxxxxxxxxxx> wrote:
On Fri, Feb 20, 2015 at 04:01:55PM -0600, Rob Herring wrote:
[...]
+= Data consumers =
+
+Required properties:
+
+eeproms: List of phandle and data cell specifier triplet, one triplet
+ for each data cell the device might be interested in. The
+ triplet consists of the phandle to the eeprom provider, then
+ the offset in byte within that storage device, and the length
+ in byte of the data we care about.
The problem with this is it assumes you know who the consumer is and
that it is a DT node. For example, how would you describe a serial
number?
Correct me if I miss understood.
Is serial number any different?
Am hoping that the eeprom consumer would be aware of offset and size of
serial number in the eeprom
Cant the consumer do:
eeprom-consumer {
eeproms = <&at24 0 4>;
eeprom-names = "device-serial-number";
Yes, but who is "eeprom-consumer"?
Any device that should lookup values in one of the EEPROM.
DT nodes generally describe a h/w block, but it this case, the
consumer depends on the OS, not the h/w.
Not really, or at least, not more than any similar binding we
currently have.
The fact that a MAC-address for the first ethernet chip is stored at a
given offset in a given eeprom has nothing to do with the OS.
So MAC address would be a valid use to link to a h/w device, but there
are certainly cases that don't correspond to a device.
I'm not saying you can't describe where things are, but I don't
think you should imply who is the consumer and doing so is
unnecessarily complicated.
If you only take the case of a serial number, indeed. If you take
other usage into account, you can't really do without it.
Also, the layout of EEPROM is likely very much platform specific.
Indeed, which is why it should be in the DT.
Agreed. I'm not saying the layout should not be.
Some could have a more complex structure perhaps with key ids and
linked list structure.
Then just request the size of the whole list, and parse it afterwards
in your driver?
I would do something more simple that is just a list of keys and their
location like this:
device-serial-number = <start size>;
key1 = <start size>;
key2 = <start size>;
I'm sorry, but what's the difference?
It can describe the layout completely whether the fields are tied to a
h/w device or not.
What I would like to see here is the entire layout described covering
both types of fields.
I was thinking the DT might be like this on the provider side:
qfprom@1000000 {
reg = <0x1000000 0x1000>;
ranges = <0 0x1000000 0x1000>;
compatible = "qcom,qfprom-msm8960"
pvs-data: pvs-data@40 {
compatible = "qcom,pvs-a";
These compatibles could be optional. As it might not be required for
devices which are simple and do not require any special interpretation
of eeprom data.
reg = <0x40 0x20>,
#eeprom-cells = <0>;
Do we still need eeprom-cells if we are moving to use reg property here?
};
tsens-data: tmdata@10 {
compatible = "qcom,tsens-data-msm8960";
reg = <0x10 4>, <0x16 4>;
#eeprom-cells = <0>;
};
serial-number: serial@50 {
compatible = "qcom,serial-msm8960";
reg = <0x50 4>, <0x60 4>;
#eeprom-cells = <0>;
};
};
and then on the consumer side:
device {
eeproms = <&serial-number>;
eeprom-names = "soc-rev-id";
};
Yes, this looks good, and Sasha was also recommending something on these
lines too. Also this addresses the use cases like serial number too.
This would solve a problem where the consumer device is some standard
off-the-shelf IP block that needs to get some SoC specific calibration
data from the eeprom. I may want to interpret the bits differently
depending on which eeprom is connected to my SoC. Sometimes that data
format may be the same across many variations of the SoC (e.g. the
qcom,pvs-a node) or it may be completely different for a given SoC (e.g.
qcom,serial-msm8960 node). I imagine for other SoCs out there it could
be different depending on which eeprom the board manufacturer decides to
wire onto their board and how they choose to program the data.
So this is where I think the eeprom-cells and offset + length starts to
fall apart. It forces us to make up a bunch of different compatible
strings for our consumer device just so that we can parse the eeprom
that we decided to use for some SoC/board specific data. Instead I'd
like to see some framework that expresses exactly which eeprom is on my
board and how to interpret the bits in a way that doesn't require me to
keep refining the compatible string for my generic IP block.
I worry that if we put all those details in DT we'll end up having to
describe individual bits like serial-number-bit-2, serial-number-bit-3,
etc. because sometimes these pieces of data are scattered all around the
eeprom and aren't contiguous or aligned on a byte boundary. It may be
easier to just have a way to express that this is an eeprom with this
specific layout and my device has data stored in there. Then the driver
can be told what layout it is (via compatible or some other string based
means if we're not using DT?) and match that up with some driver data if
it needs to know how to understand the bits it can read with the
eeprom_read() API.
Yes this sounds more sensible to let the consumer driver interpret the
eeprom data than the eeprom framework itself.
--srini
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/