[PATCH] i2c: fsi: Create busses for all ports

[Eddie James]

On 6/4/19 1:14 AM, Oliver wrote:
> On Tue, Jun 4, 2019 at 12:15 AM Eddie James <eajames at linux.ibm.com> wrote:
>>
>> On 6/3/19 12:57 AM, Oliver O'Halloran wrote:
>>> Currently we only create an I2C bus for the ports listed in the
>>> device-tree for that master. There's no real reason for this since
>>> we can discover the number of ports the master supports by looking
>>> at the port_max field of the status register.
>>>
>>> This patch re-works the bus add logic so that we always create buses
>>> for each port, unless the bus is marked as unavailable in the DT. This
>>> is useful since it ensures that all the buses provided by the CFAM I2C
>>> master are accessible to debug tools.
>>>
>>> Cc: Eddie James <eajames at linux.vnet.ibm.com>
>>> Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
>>> ---
>>>    drivers/i2c/busses/i2c-fsi.c | 30 +++++++++++++++++++++++++-----
>>>    1 file changed, 25 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
>>> index 1e2be2219a60..59a76c6e31ad 100644
>>> --- a/drivers/i2c/busses/i2c-fsi.c
>>> +++ b/drivers/i2c/busses/i2c-fsi.c
>>> @@ -658,13 +658,27 @@ static const struct i2c_algorithm fsi_i2c_algorithm = {
>>>        .functionality = fsi_i2c_functionality,
>>>    };
>>>
>>> +static device_node *fsi_i2c_find_port_of_node(struct device_node *master,
>>> +                                           int port)
> Turns out I had a pile of compile fixes staged but not committed so
> this patch is totally broken. Oops.
>
>>> +{
>>> +     struct device_node *np;
>>> +
>>> +     for_each_child_of_node(fsi, np) {
>>> +             rc = of_property_read_u32(np, "reg", &port_no);
>>> +             if (!rc && port_no == port)
>>> +                     return np;
>>> +     }
>>> +
>>> +     return NULL;
>>> +}
>>> +
>>>    static int fsi_i2c_probe(struct device *dev)
>>>    {
>>>        struct fsi_i2c_master *i2c;
>>>        struct fsi_i2c_port *port;
>>>        struct device_node *np;
>>> +     u32 port_no, ports, stat;
>>>        int rc;
>>> -     u32 port_no;
>>>
>>>        i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
>>>        if (!i2c)
>>> @@ -678,10 +692,16 @@ static int fsi_i2c_probe(struct device *dev)
>>>        if (rc)
>>>                return rc;
>>>
>>> -     /* Add adapter for each i2c port of the master. */
>>> -     for_each_available_child_of_node(dev->of_node, np) {
>>> -             rc = of_property_read_u32(np, "reg", &port_no);
>>> -             if (rc || port_no > USHRT_MAX)
>>> +     rc = fsi_i2c_read_reg(i2c->fsi, I2C_FSI_STAT, &state);
>>> +     if (rc)
>>> +             return rc;
>>> +
>>> +     ports = FIELD_GET(I2C_STAT_MAX_PORT, stat);
>>> +     dev_dbg(dev, "I2C master has %d ports\n", ports);
>>
>> Thanks for the patch Oliver. This looks great except some older CFAM
>> types don't report the max port number, in which case this would not
>> probe up any ports. So we probably need a fallback to dts if the max
>> ports is 0.
> Hmm, The oldest CFAM spec I could find was v1.2 which is from the p6
> era and it includes the MAX_PORT field. When I was checking the spec I
> noticed that I mis-interpreted the meaning of MAX_PORT. It's actually
> the largest value you can write into the port field of the mode
> register rather than the number of ports the master supports. So zero
> is a valid value for MAX_PORT that you would see if the master only
> has one port.


Yep, now that I look at the specs too, that is correct.


>
> Do you know if the old masters only had one port? If not, do you know
> what version (from the ext status reg) of the master doesn't support
> the max_port field?


I used to have some more up-to-date specs but I can't seem to find 
them... I think I see what's going on. Some versions of the CFAM have 
the max port, or "upper threshold for ports" at bits 16-19, while others 
have that information at 9-15 or 12-15... I'm not sure we can reliably 
determine where/what that number will be. I'm open to suggestions!


Thanks,

Eddie



>
>
>> Thanks,
>>
>> Eddie
>>
>>
>>> +
>>> +     for (port_no = 0; port_no < ports; port_no++) {
>>> +             np = fsi_i2c_find_port_of_node(dev.of_node, port_no);
>>> +             if (np && !of_device_is_available(np))
>>>                        continue;
>>>
>>>                port = kzalloc(sizeof(*port), GFP_KERNEL);