mbox series

[v2,0/4] eeprom: ee1004: Enable devices on multiple busses

Message ID 20230321151642.461618-1-eajames@linux.ibm.com
Headers show
Series eeprom: ee1004: Enable devices on multiple busses | expand

Message

Eddie James March 21, 2023, 3:16 p.m. UTC 
The driver previously prevented probing devices on more than one
bus due to locking constraints with the special page addresses. This
constraint can be removed by allocating a reference-counted bus
structure containing the lock, rather than using global variables.
In addition, add devicetree bindings for the EE1004 driver for the
AT30TSE device and add the devices to the Bonnell BMC system.

Changes since v1:
 - Add the devicetree changes

Eddie James (4):
  eeprom: ee1004: Enable devices on multiple busses
  doc: Add Atmel AT30TSE serial eeprom
  eeprom: ee1004: Add devicetree binding
  ARM: dts: aspeed: bonnell: Add DIMM SPD

 .../devicetree/bindings/trivial-devices.yaml  |   2 +
 arch/arm/boot/dts/aspeed-bmc-ibm-bonnell.dts  |  20 ++
 drivers/misc/eeprom/ee1004.c                  | 182 +++++++++++-------
 3 files changed, 135 insertions(+), 69 deletions(-)

Comments

Krzysztof Kozlowski March 21, 2023, 3:20 p.m. UTC | #1 
On 21/03/2023 16:16, Eddie James wrote:
> Add an OF match table for devicetree instantiation of EE1004
> devices.

Subject: There is no device tree binding here. You add OF matching (or
support) to the driver.

> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  drivers/misc/eeprom/ee1004.c | 7 +++++++
>  1 file changed, 7 insertions(+)

Best regards,
Krzysztof
Rob Herring March 21, 2023, 3:39 p.m. UTC | #2 
On Tue, Mar 21, 2023 at 10:17 AM Eddie James <eajames@linux.ibm.com> wrote:
>
> The driver previously prevented probing devices on more than one
> bus due to locking constraints with the special page addresses. This
> constraint can be removed by allocating a reference-counted bus
> structure containing the lock, rather than using global variables.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  drivers/misc/eeprom/ee1004.c | 175 +++++++++++++++++++++--------------
>  1 file changed, 106 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
> index c8c6deb7ed89..950813821087 100644
> --- a/drivers/misc/eeprom/ee1004.c
> +++ b/drivers/misc/eeprom/ee1004.c
> @@ -9,12 +9,15 @@
>   * Copyright (C) 2008 Wolfram Sang, Pengutronix
>   */
>
> +#include <linux/err.h>
>  #include <linux/i2c.h>
>  #include <linux/init.h>
>  #include <linux/kernel.h>
> +#include <linux/list.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/of_device.h>

What do you need from here? I don't see anything.

of_device.h is a mess of implicit includes which I'm currently trying
to detangle. See the ~13 year old comment in it about removing
of_platform.h include. When I'm done, pretty much only bus
implementations should include of_device.h.

Rob
Eddie James March 21, 2023, 3:45 p.m. UTC | #3 
On 3/21/23 10:39, Rob Herring wrote:
> On Tue, Mar 21, 2023 at 10:17 AM Eddie James <eajames@linux.ibm.com> wrote:
>> The driver previously prevented probing devices on more than one
>> bus due to locking constraints with the special page addresses. This
>> constraint can be removed by allocating a reference-counted bus
>> structure containing the lock, rather than using global variables.
>>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>>   drivers/misc/eeprom/ee1004.c | 175 +++++++++++++++++++++--------------
>>   1 file changed, 106 insertions(+), 69 deletions(-)
>>
>> diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
>> index c8c6deb7ed89..950813821087 100644
>> --- a/drivers/misc/eeprom/ee1004.c
>> +++ b/drivers/misc/eeprom/ee1004.c
>> @@ -9,12 +9,15 @@
>>    * Copyright (C) 2008 Wolfram Sang, Pengutronix
>>    */
>>
>> +#include <linux/err.h>
>>   #include <linux/i2c.h>
>>   #include <linux/init.h>
>>   #include <linux/kernel.h>
>> +#include <linux/list.h>
>>   #include <linux/mod_devicetable.h>
>>   #include <linux/module.h>
>>   #include <linux/mutex.h>
>> +#include <linux/of_device.h>
> What do you need from here? I don't see anything.
>
> of_device.h is a mess of implicit includes which I'm currently trying
> to detangle. See the ~13 year old comment in it about removing
> of_platform.h include. When I'm done, pretty much only bus
> implementations should include of_device.h.


You're right, I mistakenly thought I needed it for of_device_id. I'll 
remove it in v3.

Thanks,

Eddie


>
> Rob