Re: [PATCH v7 2/8] i2c: muxes: add support for tsd,mule-i2c multiplexer

[Andi Shyti]

Hi Farouk,

On Wed, Sep 04, 2024 at 12:23:56PM GMT, Farouk Bouabid wrote:
> On 03.09.24 17:13, Andi Shyti wrote:
> > > Theobroma Systems Mule is an MCU that emulates a set of I2C devices,
> > > among which an amc6821 and devices that are reachable through an I2C-mux.
> > > The devices on the mux can be selected by writing the appropriate device
> > > number to an I2C config register (amc6821 reg 0xff).
> > > 
> > > This driver is expected to be probed as a platform device with amc6821
> > > as its parent i2c device.
> > > 
> > > Add support for the mule-i2c-mux platform driver. The amc6821 driver
> > Along the driver I expressed some concern about the prefixes.
> > 
> > You should avoid prefixes such as mux_* or MUX_* because they
> > don't belong to your driver. You should always use your driver's
> > name:
> > 
> >   1. mule_*
> >   2. mule_mux_*
> >   3. mule_i2c_mux_*
> > 
> > You have used the 3rd, I'd rather prefer the 1st. Because when
> > you are in i2c/muxex/ it's implied that you are an i2c mux
> > device. But it's a matter of personal taste.
> > 
> 
> Are you here referring to the commit log, module name or function prefixes ?
> (because  later you suggested that we use "mule_i2c_mux_*" for functions)

I made a general comment that applies to all the functions,
defines, and global variables you've made here.

My suggestion to use mule_i2c_mux_* is based on the fact that
it's the most commonly used prefix in your code, but you don't
necessarily need to use it. That's why I listed a few options.

> "Mule" is a chip that requires multiple drivers that will be added later on,
> and I suppose we don't want conflict with other module names ?

It's an unwritten rule that you should avoid using overly generic
terms as prefixes in your driver, like "smbus_read()" or
"i2c_read()". Instead, you should incorporate to the prefix the
chip identifier as named by the vendor. If this device is called
'Theobroma Systems Mule,' you should stick to that naming as much
as possible.

Using the correct prefix might seem like overkill, but it's
essential for debugging, grepping, and avoiding conflicts in
cases where other developers haven’t used unique identifiers in
their modules.

Lastly, if you're working within the i2c/muxes directory, you can
omit the 'mux' prefix. It’s already clear that you're working
with an I2C mux device.

Thanks,
Andi