On Tue, May 31, 2022 at 3:55 PM Cosmin Tanislav <demonsingur@xxxxxxxxx> wrote:
>
> From: Cosmin Tanislav <cosmin.tanislav@xxxxxxxxxx>
>
> SPI can only use 5 address bits, since one bit is reserved for
> specifying R/W and 2 bits are used to specify the UART port.
> To access registers that have addresses past 0x1F, an extended
> register space can be enabled by writing to the GlobalCommand
> register (address 0x1F).
>
> I2C uses 8 address bits. The R/W bit is placed in the slave
> address, and so is the UART port. Because of this, registers
> that have addresses higher than 0x1F can be accessed normally.
>
> To access the RevID register, on SPI, 0xCE must be written to
> the 0x1F address to enable the extended register space, after
> which the RevID register is accessible at address 0x5. 0xCD
> must be written to the 0x1F address to disable the extended
> register space.
>
> On I2C, the RevID register is accessible at address 0x25.
>
> Create an interface config struct, and add a method for
> toggling the extended register space and a member for the RevId
> register address. Implement these for SPI.
...
> struct max310x_port {
> const struct max310x_devtype *devtype;
> + const struct max310x_if_cfg *if_cfg;
> struct regmap *regmap;
I believe the most used pointer is regmap and putting it to be a first
member will make pointer arithmetic no-op at compile time. That said,
adding new member is better after this one.
> struct clk *clk;
...
> + ret = s->if_cfg->set_ext_reg_en(dev, true);
It sounds like a voodoo speech. Can we name the callback better?
->extended_reg_enable() ?
> if (ret)
> return ret;
...
> static int max310x_probe(struct device *dev, const struct max310x_devtype *devtype,
> + const struct max310x_if_cfg *if_cfg,
> struct regmap **regmaps, int irq)
It should be commented on the other patch, but since I can't see it in
my mailbox (yet) I put it here. So,
looking into usage of regmaps parameter it logically should be declared as
struct regmap *regmaps[]
(yes, I know that there is no difference for the compiler, but code
human reader).
...
> - return max310x_probe(&spi->dev, devtype, regmaps, spi->irq);
> + return max310x_probe(&spi->dev, devtype, &max310x_spi_if_cfg, regmaps,
> + spi->irq);
Can still be on one line, no?
--
With Best Regards,
Andy Shevchenko
