Re: [PATCH v7 2/2] i2c: npcm: Add Nuvoton NPCM I2C controller driver

[Tali Perry <tali.perry1@gmail.com>]

Hi Wolfram,

Thanks for your comments.

The NPCM7XX BMC I2C\SMB controller HW module supports both SMB and I2C.
It's main features are:
1. Supports Fast-Mode (400 KHz clock) I2C and Fast-Mode-plus (1 MHz clock) I2C
2. Supports the ‘fairness’ arbitration protocol defined by the MCTP
SMBus/I2C Transport Binding Specification v1.0.0
3. 32KB packets : this is an I2C spec limitation. The HW has no limit
on packets size. It has a 16 bytes FIFO which can be reloaded over and
over.
4. w\o size byte (for SMB block protocol).
5. Both master and slave. It can also replace modes in run time
(requirement for IPMB and MCTP).
6. Bus timing is selected to support both specs.

Originally the HW spec stated SMB everywhere .

Should I rename the SMB to I2C all over the driver?

Thanks,
Tali Perry


On Tue, Nov 26, 2019 at 8:47 AM Tali Perry <tali.perry1@gmail.com> wrote:
>
> Hi Wolfram,
>
> Thanks for your comments.
>
> The NPCM7XX BMC I2C\SMB controller HW module supports both SMB and I2C.
> It's main features are:
> 1. Supports Fast-Mode (400 KHz clock) I2C and Fast-Mode-plus (1 MHz clock) I2C
> 2. Supports the ‘fairness’ arbitration protocol defined by the MCTP SMBus/I2C Transport Binding Specification v1.0.0
> 3. 32KB packets : this is an I2C spec limitation. The HW has no limit on packets size. It has a 16 bytes FIFO which can be reloaded over and over.
> 4. w\o size byte (for SMB block protocol).
> 5. Both master and slave. It can also replace modes in run time (requirement for IPMB and MCTP).
> 6. Bus timing is selected to support both specs.
>
> Originally the HW spec stated SMB everywhere .
>
> Should I rename the SMB to I2C all over the driver?
>
> Thanks,
> Tali Perry
>
>
> On Mon, Nov 25, 2019 at 5:16 PM Wolfram Sang <wsa@the-dreams.de> wrote:
>>
>> On Thu, Nov 21, 2019 at 11:53:50AM +0200, Tali Perry wrote:
>> > Add Nuvoton NPCM BMC i2c controller driver.
>> >
>> > Signed-off-by: Tali Perry <tali.perry1@gmail.com>
>>
>> Looking at all this SMB_* naming of the registers and also the quirks,
>> this looks more like an SMBUS controller to me?
>>
>> > +     // currently I2C slave IF only supports single byte operations.
>> > +     // in order to utilyze the npcm HW FIFO, the driver will ask for 16bytes
>> > +     // at a time, pack them in buffer, and then transmit them all together
>> > +     // to the FIFO and onward to the bus .
>> > +     // NACK on read will be once reached to bus->adap->quirks->max_read_len
>> > +     // sending a NACK whever the backend requests for it is not supported.
>>
>> This for example...
>>
>> > +static const struct i2c_adapter_quirks npcm_i2c_quirks = {
>> > +     .max_read_len = 32768,
>> > +     .max_write_len = 32768,
>> > +     .max_num_msgs = 2,
>> > +     .flags = I2C_AQ_COMB_WRITE_THEN_READ
>> > +};
>>
>> ... and this. Like SMBus with the only exception of being able to send
>> 32K in a row. Or?
>>