RE: [PATCH mfd v2] mfd: Add Renesas Synchronization Management Unit (SMU) support

[Min Li]

> > diff --git a/drivers/mfd/rsmu_spi.c b/drivers/mfd/rsmu_spi.c
> > new file mode 100644
> > index 0000000..f3a087b
> > --- /dev/null
> > +++ b/drivers/mfd/rsmu_spi.c
> > @@ -0,0 +1,265 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * SPI driver for the IDT ClockMatrix(TM) and 82P33xxx families of
> > + * timing and synchronization devices.
> > + *
> > + * Copyright (C) 2019 Integrated Device Technology, Inc., a Renesas
> Company.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/slab.h>
> > +#include <linux/spi/spi.h>
> > +#include <linux/regmap.h>
> > +#include <linux/of.h>
> > +#include <linux/mfd/core.h>
> > +#include <linux/mfd/rsmu.h>
> > +#include "rsmu_private.h"
> > +
> > +/*
> > + * 16-bit register address: the lower 7 bits of the register address come
> > + * from the offset addr byte and the upper 9 bits come from the page
> register.
> > + */
> > +#define	RSMU_CM_PAGE_ADDR		0x7C
> > +#define	RSMU_SABRE_PAGE_ADDR		0x7F
> > +#define	RSMU_HIGHER_ADDR_MASK		0xFF80
> > +#define	RSMU_HIGHER_ADDR_SHIFT		7
> > +#define	RSMU_LOWER_ADDR_MASK		0x7F
> > +
> > +static int rsmu_read_device(struct rsmu_dev *rsmu, u8 reg, u8 *buf,
> u16 bytes)
> > +{
> > +	struct spi_device *client = to_spi_device(rsmu->dev);
> > +	struct spi_transfer xfer = {0};
> > +	struct spi_message msg;
> > +	u8 cmd[256] = {0};
> > +	u8 rsp[256] = {0};
> > +	int ret;
> > +
> > +	cmd[0] = reg | 0x80;
> > +	xfer.rx_buf = rsp;
> > +	xfer.len = bytes + 1;
> > +	xfer.tx_buf = cmd;
> > +	xfer.bits_per_word = client->bits_per_word;
> > +	xfer.speed_hz = client->max_speed_hz;
> > +
> > +	spi_message_init(&msg);
> > +	spi_message_add_tail(&xfer, &msg);
> > +
> > +	ret = spi_sync(client, &msg);
> > +	if (ret >= 0)
> > +		memcpy(buf, &rsp[1], xfer.len-1);
> 
> What's at rsp[0]?  Worth a comment?
> 

I don't really know. This code is being used as is and tested working.
Can I find out and add the comment later?

> 
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/linux/mfd/idt82p33_reg.h
> b/include/linux/mfd/idt82p33_reg.h
> > new file mode 100644
> > index 0000000..fb41ab0
> > --- /dev/null
> > +++ b/include/linux/mfd/idt82p33_reg.h
> > @@ -0,0 +1,112 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/* idt82p33_reg.h
> 
> Remove the filename, they have a habit of becoming out of date.
> 
> > + *
> > + * Register Map - AN888_SMUforIEEE_SynchEther_82P33xxx_RevH.pdf
> > + *
> > + */
> 
> Copyright.
> 

Sorry, what do you mean by this?