The i2c-slave-eeprom driver emulates at24 class eeprom devices,
which come initialized with all 1s. Do the same in the software
emulation.
Signed-off-by: Patrick Williams <[email protected]>
---
drivers/i2c/i2c-slave-eeprom.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/i2c/i2c-slave-eeprom.c b/drivers/i2c/i2c-slave-eeprom.c
index db9763cb4dae..efee56106251 100644
--- a/drivers/i2c/i2c-slave-eeprom.c
+++ b/drivers/i2c/i2c-slave-eeprom.c
@@ -131,6 +131,8 @@ static int i2c_slave_eeprom_probe(struct i2c_client *client, const struct i2c_de
if (!eeprom)
return -ENOMEM;
+ memset(eeprom->buffer, 0xFF, size);
+
eeprom->idx_write_cnt = 0;
eeprom->num_address_bytes = flag_addr16 ? 2 : 1;
eeprom->address_mask = size - 1;
--
2.17.2 (Apple Git-113)
Add support for emulating the following EEPROMs:
* 24c01 - 1024 bit
* 24c128 - 128k bit
* 24c256 - 256k bit
* 24c512 - 512k bit
The flag bits in the device id were shifted up 1 bit to make
room for saving the 24c512's size. 24c512 uses the full 16-bit
address space of a 2-byte addressable EEPROM.
Signed-off-by: Patrick Williams <[email protected]>
---
drivers/i2c/i2c-slave-eeprom.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/i2c-slave-eeprom.c b/drivers/i2c/i2c-slave-eeprom.c
index efee56106251..65419441313b 100644
--- a/drivers/i2c/i2c-slave-eeprom.c
+++ b/drivers/i2c/i2c-slave-eeprom.c
@@ -37,9 +37,9 @@ struct eeprom_data {
u8 buffer[];
};
-#define I2C_SLAVE_BYTELEN GENMASK(15, 0)
-#define I2C_SLAVE_FLAG_ADDR16 BIT(16)
-#define I2C_SLAVE_FLAG_RO BIT(17)
+#define I2C_SLAVE_BYTELEN GENMASK(16, 0)
+#define I2C_SLAVE_FLAG_ADDR16 BIT(17)
+#define I2C_SLAVE_FLAG_RO BIT(18)
#define I2C_SLAVE_DEVICE_MAGIC(_len, _flags) ((_flags) | (_len))
static int i2c_slave_eeprom_slave_cb(struct i2c_client *client,
@@ -171,12 +171,20 @@ static int i2c_slave_eeprom_remove(struct i2c_client *client)
}
static const struct i2c_device_id i2c_slave_eeprom_id[] = {
+ { "slave-24c01", I2C_SLAVE_DEVICE_MAGIC(1024 / 8, 0) },
+ { "slave-24c01ro", I2C_SLAVE_DEVICE_MAGIC(1024 / 8, I2C_SLAVE_FLAG_RO) },
{ "slave-24c02", I2C_SLAVE_DEVICE_MAGIC(2048 / 8, 0) },
{ "slave-24c02ro", I2C_SLAVE_DEVICE_MAGIC(2048 / 8, I2C_SLAVE_FLAG_RO) },
{ "slave-24c32", I2C_SLAVE_DEVICE_MAGIC(32768 / 8, I2C_SLAVE_FLAG_ADDR16) },
{ "slave-24c32ro", I2C_SLAVE_DEVICE_MAGIC(32768 / 8, I2C_SLAVE_FLAG_ADDR16 | I2C_SLAVE_FLAG_RO) },
{ "slave-24c64", I2C_SLAVE_DEVICE_MAGIC(65536 / 8, I2C_SLAVE_FLAG_ADDR16) },
{ "slave-24c64ro", I2C_SLAVE_DEVICE_MAGIC(65536 / 8, I2C_SLAVE_FLAG_ADDR16 | I2C_SLAVE_FLAG_RO) },
+ { "slave-24c128", I2C_SLAVE_DEVICE_MAGIC(131072 / 8, I2C_SLAVE_FLAG_ADDR16) },
+ { "slave-24c128ro", I2C_SLAVE_DEVICE_MAGIC(131072 / 8, I2C_SLAVE_FLAG_ADDR16 | I2C_SLAVE_FLAG_RO) },
+ { "slave-24c256", I2C_SLAVE_DEVICE_MAGIC(262144 / 8, I2C_SLAVE_FLAG_ADDR16) },
+ { "slave-24c256ro", I2C_SLAVE_DEVICE_MAGIC(262144 / 8, I2C_SLAVE_FLAG_ADDR16 | I2C_SLAVE_FLAG_RO) },
+ { "slave-24c512", I2C_SLAVE_DEVICE_MAGIC(524288 / 8, I2C_SLAVE_FLAG_ADDR16) },
+ { "slave-24c512ro", I2C_SLAVE_DEVICE_MAGIC(524288 / 8, I2C_SLAVE_FLAG_ADDR16 | I2C_SLAVE_FLAG_RO) },
{ }
};
MODULE_DEVICE_TABLE(i2c, i2c_slave_eeprom_id);
--
2.17.2 (Apple Git-113)
Hi,
I sent in another patch earlier that added support for specifying a file
in devicetree to initilize the eeprom from, corresponding to the case of
pre-flashed eeprom. Maybe these two patches should be merged so this
initialization is only done if no file is specified?
/BA
On 10/1/19 6:40 PM, Patrick Williams wrote:
> The i2c-slave-eeprom driver emulates at24 class eeprom devices,
> which come initialized with all 1s. Do the same in the software
> emulation.
>
> Signed-off-by: Patrick Williams <[email protected]>
> ---
> drivers/i2c/i2c-slave-eeprom.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/i2c/i2c-slave-eeprom.c b/drivers/i2c/i2c-slave-eeprom.c
> index db9763cb4dae..efee56106251 100644
> --- a/drivers/i2c/i2c-slave-eeprom.c
> +++ b/drivers/i2c/i2c-slave-eeprom.c
> @@ -131,6 +131,8 @@ static int i2c_slave_eeprom_probe(struct i2c_client *client, const struct i2c_de
> if (!eeprom)
> return -ENOMEM;
>
> + memset(eeprom->buffer, 0xFF, size);
> +
> eeprom->idx_write_cnt = 0;
> eeprom->num_address_bytes = flag_addr16 ? 2 : 1;
> eeprom->address_mask = size - 1;
On Wed, Oct 02, 2019 at 08:20:53AM +0200, Bjorn Ardo wrote:
> Hi,
>
>
> I sent in another patch earlier that added support for specifying a file in
> devicetree to initilize the eeprom from, corresponding to the case of
> pre-flashed eeprom. Maybe these two patches should be merged so this
> initialization is only done if no file is specified?
Yes, I agree.
> /BA
>
> On 10/1/19 6:40 PM, Patrick Williams wrote:
> > The i2c-slave-eeprom driver emulates at24 class eeprom devices,
> > which come initialized with all 1s. Do the same in the software
> > emulation.
> >
> > Signed-off-by: Patrick Williams <[email protected]>
> > ---
> > drivers/i2c/i2c-slave-eeprom.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/i2c/i2c-slave-eeprom.c b/drivers/i2c/i2c-slave-eeprom.c
> > index db9763cb4dae..efee56106251 100644
> > --- a/drivers/i2c/i2c-slave-eeprom.c
> > +++ b/drivers/i2c/i2c-slave-eeprom.c
> > @@ -131,6 +131,8 @@ static int i2c_slave_eeprom_probe(struct i2c_client *client, const struct i2c_de
> > if (!eeprom)
> > return -ENOMEM;
> > + memset(eeprom->buffer, 0xFF, size);
> > +
> > eeprom->idx_write_cnt = 0;
> > eeprom->num_address_bytes = flag_addr16 ? 2 : 1;
> > eeprom->address_mask = size - 1;
On Tue, Oct 01, 2019 at 11:40:06AM -0500, Patrick Williams wrote:
> Add support for emulating the following EEPROMs:
> * 24c01 - 1024 bit
> * 24c128 - 128k bit
> * 24c256 - 256k bit
> * 24c512 - 512k bit
>
> The flag bits in the device id were shifted up 1 bit to make
> room for saving the 24c512's size. 24c512 uses the full 16-bit
> address space of a 2-byte addressable EEPROM.
>
> Signed-off-by: Patrick Williams <[email protected]>
Do you really need them or is it just nice to have?
I am undecided. I definately don't want all the EEPROM types which
exist, but the full 16 bit address range makes sense...
More opinions welcome.
On Mon, Apr 20, 2020 at 06:43:49PM +0200, Wolfram Sang wrote:
> On Wed, Oct 02, 2019 at 08:20:53AM +0200, Bjorn Ardo wrote:
> > Hi,
> >
> >
> > I sent in another patch earlier that added support for specifying a file in
> > devicetree to initilize the eeprom from, corresponding to the case of
> > pre-flashed eeprom. Maybe these two patches should be merged so this
> > initialization is only done if no file is specified?
>
> Yes, I agree.
>
I've been meaning to get back to this... I'll rebase the
commit on the latest tree.
--
Patrick Williams
On Mon, Apr 20, 2020 at 06:46:19PM +0200, Wolfram Sang wrote:
> On Tue, Oct 01, 2019 at 11:40:06AM -0500, Patrick Williams wrote:
> > Add support for emulating the following EEPROMs:
> > * 24c01 - 1024 bit
> > * 24c128 - 128k bit
> > * 24c256 - 256k bit
> > * 24c512 - 512k bit
> >
> > The flag bits in the device id were shifted up 1 bit to make
> > room for saving the 24c512's size. 24c512 uses the full 16-bit
> > address space of a 2-byte addressable EEPROM.
> >
> > Signed-off-by: Patrick Williams <[email protected]>
>
> Do you really need them or is it just nice to have?
>
> I am undecided. I definately don't want all the EEPROM types which
> exist, but the full 16 bit address range makes sense...
>
> More opinions welcome.
>
I don't remember exactly which ones we needed (and I am no longer at
Amazon), but it was pretty trivial to add them all to the table so I
went ahead and did it. As long as we had one of the 2-byte addressable
EEPROMs, anything else necessary could be handleded as a small
out-of-tree patch.
--
Patrick Williams
On Mon, Apr 20, 2020 at 06:43:49PM +0200, Wolfram Sang wrote:
> On Wed, Oct 02, 2019 at 08:20:53AM +0200, Bjorn Ardo wrote:
> > Hi,
> >
> >
> > I sent in another patch earlier that added support for specifying a file in
> > devicetree to initilize the eeprom from, corresponding to the case of
> > pre-flashed eeprom. Maybe these two patches should be merged so this
> > initialization is only done if no file is specified?
>
> Yes, I agree.
>
It looks like Bjorn's referenced patches are still unmerged also?
--
Patrick Williams
On Mon, Apr 20, 2020 at 03:31:46PM -0500, Patrick Williams wrote:
> On Mon, Apr 20, 2020 at 06:43:49PM +0200, Wolfram Sang wrote:
> > On Wed, Oct 02, 2019 at 08:20:53AM +0200, Bjorn Ardo wrote:
> > > Hi,
> > >
> > >
> > > I sent in another patch earlier that added support for specifying a file in
> > > devicetree to initilize the eeprom from, corresponding to the case of
> > > pre-flashed eeprom. Maybe these two patches should be merged so this
> > > initialization is only done if no file is specified?
> >
> > Yes, I agree.
> >
>
> It looks like Bjorn's referenced patches are still unmerged also?
Yes, it might be easiest if he merges your patch (with attribution) into
the else branch of his fw-load patch.
On 4/20/20 10:53 PM, Wolfram Sang wrote:
> On Mon, Apr 20, 2020 at 03:31:46PM -0500, Patrick Williams wrote:
>> On Mon, Apr 20, 2020 at 06:43:49PM +0200, Wolfram Sang wrote:
>>> On Wed, Oct 02, 2019 at 08:20:53AM +0200, Bjorn Ardo wrote:
>>>> Hi,
>>>>
>>>>
>>>> I sent in another patch earlier that added support for specifying a file in
>>>> devicetree to initilize the eeprom from, corresponding to the case of
>>>> pre-flashed eeprom. Maybe these two patches should be merged so this
>>>> initialization is only done if no file is specified?
>>> Yes, I agree.
>>>
>> It looks like Bjorn's referenced patches are still unmerged also?
> Yes, it might be easiest if he merges your patch (with attribution) into
> the else branch of his fw-load patch.
>
OK, so to summarize, I should update my patch to use
device_property_read_string() instead and also init the memory to 0XFF
if no file is present. And change name of the function to
i2c_slave_init_eeprom_data.
I will look into that and let you know once I'm done.
/BA
> > Yes, it might be easiest if he merges your patch (with attribution) into
> > the else branch of his fw-load patch.
> >
>
> OK, so to summarize, I should update my patch to use
> device_property_read_string() instead and also init the memory to 0XFF if no
> file is present. And change name of the function to
Or something else went wrong.
> i2c_slave_init_eeprom_data.
Yes, that is my idea. You also need to replace checking for an of_node
with some equivalent for device properties maybe, but that should be
easy to find out.
> I will look into that and let you know once I'm done.
Thank you!
On 4/21/20 2:16 PM, Wolfram Sang wrote:
>>> Yes, it might be easiest if he merges your patch (with attribution) into
>>> the else branch of his fw-load patch.
>>>
>> OK, so to summarize, I should update my patch to use
>> device_property_read_string() instead and also init the memory to 0XFF if no
>> file is present. And change name of the function to
> Or something else went wrong.
I did like this now: If device_property_read_string() returns a firmware
name, I use that, otherwise init to 0xFF. But if it returns a firmware
name, and for some reason I get an error when trying to load that
firmware I will not default to 0xFF, but rather fail the probe. The
logic in that is that if you actively supply a firmware name, you should
not silently get 0xFF in your eeprom. Does that sound good?
>> i2c_slave_init_eeprom_data.
> Yes, that is my idea. You also need to replace checking for an of_node
> with some equivalent for device properties maybe, but that should be
> easy to find out.
It appears to me that those kind of checks are done inside
device_property_read_string() so I can just remove them and only look at
the return value of that function.
>> I will look into that and let you know once I'm done.
> Thank you!
>
I have a patch now working on 4.14, will run some tests on it and then
try to forward-port to latest kernel och see if it works there as well.
/BA
> I did like this now: If device_property_read_string() returns a firmware
> name, I use that, otherwise init to 0xFF. But if it returns a firmware name,
> and for some reason I get an error when trying to load that firmware I will
> not default to 0xFF, but rather fail the probe. The logic in that is that if
> you actively supply a firmware name, you should not silently get 0xFF in
> your eeprom. Does that sound good?
Sounds perfect to me.
> > Yes, that is my idea. You also need to replace checking for an of_node
> > with some equivalent for device properties maybe, but that should be
> > easy to find out.
>
> It appears to me that those kind of checks are done inside
> device_property_read_string() so I can just remove them and only look at the
> return value of that function.
Even better!
> I have a patch now working on 4.14, will run some tests on it and then try
> to forward-port to latest kernel och see if it works there as well.
Looking forward to it. I will look at it right away then!
On 4/22/20 11:36 AM, Wolfram Sang wrote:
>> I have a patch now working on 4.14, will run some tests on it and then try
>> to forward-port to latest kernel och see if it works there as well.
> Looking forward to it. I will look at it right away then!
>
Patch tested on latest kernel and submitted.
/BA