Re: [PATCH v5 1/2] spi: Add Renesas R-Car Gen3 RPC-IF SPI controller driver
Hi Mason,
On Thu, Jan 17, 2019 at 7:32 AM wrote:
> > "Sergei Shtylyov"
> > 2019/01/17 上午 03:36
> > >>
> > >> [...]
> > >> > +#define RPC_WBUF 0x8000 // Write Buffer
> > >> > +#define RPC_WBUF_SIZE 256 // Write Buffer size
> > >>
> > >>I wonder if the write buffer should be in a separate "reg" prop
> > >> tuple...
> > >> Have you considered that?
> > >>
> > >
> > > I don't get your point!
> >
> >I mean that the write buffer should be a separate "reg" property
> > address/size tuple,
> > so that the RPC device has 3 memory resources. Our SPI driver used
> > this scheme, and I
> > like it.
> >
>
> I got your point but I think this RPC_WBUF@0xEE20_8000 is in the same group
> and it's size only 256 bytes, not like external address space read mode
> @0x0800_ w/ 64K bytes.
>
> Maybe Geert or Marek could comment on it, thanks.
Given the writer buffer is separate from the other registers (it's in
a different
4 KiB page, and thus may be at a different offset on future SoCs), and not
present on RZ/A1, I think it's a good idea to use a separate reg entry for it.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v5 1/2] spi: Add Renesas R-Car Gen3 RPC-IF SPI controller driver
On 01/16/2019 12:35 PM, masonccy...@mxic.com.tw wrote:
[...]
>> > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
>> > index 9f89cb1..81b4e04 100644
>> > --- a/drivers/spi/Kconfig
>> > +++ b/drivers/spi/Kconfig
[...]
>>
>> > + tristate "Renesas R-Car Gen3 RPC-IF SPI controller"
>> > + depends on ARCH_RENESAS || COMPILE_TEST
>>
>>Judging on the compilation error reported by the 0-day bot about readq(),
>> we now need to depend on 64BIT or something...
>
> I have patched RPC external address space read mode
> and driver don't need readq() now!
Good work, thank you!
>> [...]
>> > diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
>> > index f296270..3f2b2f9 100644
>> > --- a/drivers/spi/Makefile
>> > +++ b/drivers/spi/Makefile
>> [...]
>> > diff --git a/drivers/spi/spi-renesas-rpc.c b/drivers/spi/spi-renesas-rpc.c
>> > new file mode 100644
>> > index 000..1e57eb1
>> > --- /dev/null
>> > +++ b/drivers/spi/spi-renesas-rpc.c
>> > @@ -0,0 +1,787 @@
>> [...]
>> > +#define RPC_CMNCR 0x // R/W
>> > +#define RPC_CMNCR_MD BIT(31)
>> > +#define RPC_CMNCR_SFDE BIT(24) // undocumented bit but must be set
>> > +#define RPC_CMNCR_MOIIO3(val) (((val) & 0x3) << 22)
>> > +#define RPC_CMNCR_MOIIO2(val) (((val) & 0x3) << 20)
>> > +#define RPC_CMNCR_MOIIO1(val) (((val) & 0x3) << 18)
>> > +#define RPC_CMNCR_MOIIO0(val) (((val) & 0x3) << 16)
>> > +#define RPC_CMNCR_MOIIO_HIZ (RPC_CMNCR_MOIIO0(3) |
>> RPC_CMNCR_MOIIO1(3) | \
>> > + RPC_CMNCR_MOIIO2(3) | RPC_CMNCR_MOIIO3(3))
>> > +#define RPC_CMNCR_IO3FV(val) (((val) & 0x3) << 14) // undocumented bit
>> > +#define RPC_CMNCR_IO2FV(val) (((val) & 0x3) << 12) // undocumented bit
>>
>>Those are not exactly bits. I'd be happy with just // undocumented...
>>
>> [...]
>> > +#define RPC_WBUF 0x8000 // Write Buffer
>> > +#define RPC_WBUF_SIZE 256 // Write Buffer size
>>
>>I wonder if the write buffer should be in a separate "reg" prop tuple...
>> Have you considered that?
>>
>
> I don't get your point!
I mean that the write buffer should be a separate "reg" property
address/size tuple,
so that the RPC device has 3 memory resources. Our SPI driver used this scheme,
and I
like it.
>> [...]
>> > +static void rpc_spi_hw_init(struct rpc_spi *rpc)
>> > +{
>> > + //
>> > + // NOTE: The 0x260 are undocumented bits, but they must be set.
>> > + // RPC_PHYCNT_STRTIM is strobe timing adjustment bit,
>> > + // 0x0 : the delay is biggest,
>> > + // 0x1 : the delay is 2nd biggest,
>> > + // On H3 ES1.x, the value should be 0, while on others,
>> > + // the value should be 6.
>> > + //
>> > + regmap_write(rpc->regmap, RPC_PHYCNT, RPC_PHYCNT_CAL |
>> > + RPC_PHYCNT_STRTIM(6) | 0x260);
>> > +
>> > + //
>> > + // NOTE: The 0x1511144 are undocumented bits, but they must be set
>> > + // for RPC_PHYOFFSET1.
>> > + //The 0x31 are undocumented bits, but they must be set
>> > + //for RPC_PHYOFFSET2.
>> > + //
>> > + regmap_write(rpc->regmap, RPC_PHYOFFSET1,
>> > + RPC_PHYOFFSET1_DDRTMG(3) | 0x1511144);
>> > + regmap_write(rpc->regmap, RPC_PHYOFFSET2, 0x31 |
>> > + RPC_PHYOFFSET2_OCTTMG(4));
>>
>>I still would have preferred using regmap_update_bits() here...
>>
>
> This is a init() function to set up an initial value to registers.
Note that 0x31 is read only register value.
> [...]
>> > +static int rpc_spi_io_xfer(struct rpc_spi *rpc,
>> > +const void *tx_buf, void *rx_buf)
>> > +{
>> [...]
>> > + } else if (rx_buf) {
>> > + //
>> > + // RPC-IF spoils the data for the commands without an address
>> > + // phase (like RDID) in the manual mode, so we'll have to work
>> > + // around this issue by using the external address space read
>> > + // mode instead; we seem to be able to read 8 bytes at most in
>> > + // this mode though...
>> > + //
>> > + if (!(smenr & RPC_SMENR_ADE(0xf))) {
>> > + u32 nbytes = rpc->xferlen - pos;
>> > + u64 tmp;
>> > +
>> > + if (nbytes > 8)
>> > +nbytes = 8;
>> > +
>> > + regmap_update_bits(rpc->regmap, RPC_CMNCR,
>> > + RPC_CMNCR_MD, 0);
>> > + regmap_write(rpc->regmap, RPC_DRCR, 0);
>> > + regmap_write(rpc->regmap, RPC_DREAR, RPC_DREAR_EAC(1));
>> > + regmap_write(rpc->regmap, RPC_DRCMR, rpc->cmd);
>> > + regmap_write(rpc->regmap, RPC_DRDMCR, rpc->dummy);
>> > + regmap_write(rpc->regmap, RPC_DROPR, 0);
>> > + regmap_write(rpc->regmap, RPC_DRENR, rpc->smenr &
>> > + ~RPC_SMENR_SPIDE(0xf));
>>
>>The 'smenr' filed needs a more universal name -- it's written to
>> both SMENR and DRENR?
>
> I think it's ok because their bits are compatible.
Not quite, the LSBs are not compatible, as you can see from the code above.
I'd suggest smth like 'enr' or 'enable'...
> I patch external address space read mode as
Re: [PATCH v5 1/2] spi: Add Renesas R-Car Gen3 RPC-IF SPI controller driver
On 01/10/2019 01:16 PM, Boris Brezillon wrote: >>> Add a driver for Renesas R-Car Gen3 RPC-IF SPI controller. >>> >>> Signed-off-by: Mason Yang >> >>You now need to add: >> >> Signed-off-by: Sergei Shtylyov > > May I ask why? v5 includes my signed read mode workaround patch. MBR, Sergei
Re: [PATCH v5 1/2] spi: Add Renesas R-Car Gen3 RPC-IF SPI controller driver
On Wed, 9 Jan 2019 21:47:48 +0300 Sergei Shtylyov wrote: > On 01/08/2019 07:16 AM, Mason Yang wrote: > > > Add a driver for Renesas R-Car Gen3 RPC-IF SPI controller. > > > > Signed-off-by: Mason Yang > >You now need to add: > > Signed-off-by: Sergei Shtylyov May I ask why?
RE: [PATCH v5 1/2] spi: Add Renesas R-Car Gen3 RPC-IF SPI controller driver
On Wednesday, January 09, 2019, Sergei Shtylyov wrote: > > IIRC, this hardware block is also used on RZ/A, which is 32-bit. > > I'm not seeing it in the "RZ/A1H Group, RZ/A1M Group User’s Manual: > Hardware" > Rev 3.00. What SoC did you have in mind? For the RZ/A series (and RZ/T series), it is called the "SPI Multi I/O Bus Controller" (Chapter 17) I have no idea why it has a different name for the same hardware (and the same pages in the manual). The HW version in RZ/A1 does not have HyperRAM. But the HW version in RZ/A2 is the same as what is in R-Car Gen3. Chris
Re: [PATCH v5 1/2] spi: Add Renesas R-Car Gen3 RPC-IF SPI controller driver
On 1/9/19 8:04 PM, Sergei Shtylyov wrote: > On 01/09/2019 09:49 PM, Geert Uytterhoeven wrote: > Add a driver for Renesas R-Car Gen3 RPC-IF SPI controller. Signed-off-by: Mason Yang >>> >>>You now need to add: >>> >>> Signed-off-by: Sergei Shtylyov >> --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -544,6 +544,12 @@ config SPI_RSPI help SPI driver for Renesas RSPI and QSPI blocks. +config SPI_RENESAS_RPC_IF >>> >>>Since the driver is called without -IF suffix, I wouldn't use it in the >>> Kconfig name either, the following is enough: >>> + tristate "Renesas R-Car Gen3 RPC-IF SPI controller" + depends on ARCH_RENESAS || COMPILE_TEST >>> >>>Judging on the compilation error reported by the 0-day bot about readq(), >>> we now need to depend on 64BIT or something... >> >> IIRC, this hardware block is also used on RZ/A, which is 32-bit. > > I'm not seeing it in the "RZ/A1H Group, RZ/A1M Group User’s Manual: > Hardware" > Rev 3.00. What SoC did you have in mind? At least the GR Peach boots from this block, so that one :) -- Best regards, Marek Vasut
Re: [PATCH v5 1/2] spi: Add Renesas R-Car Gen3 RPC-IF SPI controller driver
On 01/09/2019 09:49 PM, Geert Uytterhoeven wrote:
>>> Add a driver for Renesas R-Car Gen3 RPC-IF SPI controller.
>>>
>>> Signed-off-by: Mason Yang
>>
>>You now need to add:
>>
>> Signed-off-by: Sergei Shtylyov
>
>>> --- a/drivers/spi/Kconfig
>>> +++ b/drivers/spi/Kconfig
>>> @@ -544,6 +544,12 @@ config SPI_RSPI
>>> help
>>> SPI driver for Renesas RSPI and QSPI blocks.
>>>
>>> +config SPI_RENESAS_RPC_IF
>>
>>Since the driver is called without -IF suffix, I wouldn't use it in the
>> Kconfig name either, the following is enough:
>>
>>> + tristate "Renesas R-Car Gen3 RPC-IF SPI controller"
>>> + depends on ARCH_RENESAS || COMPILE_TEST
>>
>>Judging on the compilation error reported by the 0-day bot about readq(),
>> we now need to depend on 64BIT or something...
>
> IIRC, this hardware block is also used on RZ/A, which is 32-bit.
I'm not seeing it in the "RZ/A1H Group, RZ/A1M Group User’s Manual: Hardware"
Rev 3.00. What SoC did you have in mind?
> Gr{oetje,eeting}s,
>
> Geert
MBR, Sergei
Re: [PATCH v5 1/2] spi: Add Renesas R-Car Gen3 RPC-IF SPI controller driver
Hi Sergei,
On Wed, Jan 9, 2019 at 7:47 PM Sergei Shtylyov
wrote:
> On 01/08/2019 07:16 AM, Mason Yang wrote:
> > Add a driver for Renesas R-Car Gen3 RPC-IF SPI controller.
> >
> > Signed-off-by: Mason Yang
>
>You now need to add:
>
> Signed-off-by: Sergei Shtylyov
> > --- a/drivers/spi/Kconfig
> > +++ b/drivers/spi/Kconfig
> > @@ -544,6 +544,12 @@ config SPI_RSPI
> > help
> > SPI driver for Renesas RSPI and QSPI blocks.
> >
> > +config SPI_RENESAS_RPC_IF
>
>Since the driver is called without -IF suffix, I wouldn't use it in the
> Kconfig name either, the following is enough:
>
> > + tristate "Renesas R-Car Gen3 RPC-IF SPI controller"
> > + depends on ARCH_RENESAS || COMPILE_TEST
>
>Judging on the compilation error reported by the 0-day bot about readq(),
> we now need to depend on 64BIT or something...
IIRC, this hardware block is also used on RZ/A, which is 32-bit.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v5 1/2] spi: Add Renesas R-Car Gen3 RPC-IF SPI controller driver
Hello!
On 01/08/2019 07:16 AM, Mason Yang wrote:
> Add a driver for Renesas R-Car Gen3 RPC-IF SPI controller.
>
> Signed-off-by: Mason Yang
You now need to add:
Signed-off-by: Sergei Shtylyov
> ---
> drivers/spi/Kconfig | 6 +
> drivers/spi/Makefile | 1 +
> drivers/spi/spi-renesas-rpc.c | 787
> ++
> 3 files changed, 794 insertions(+)
> create mode 100644 drivers/spi/spi-renesas-rpc.c
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 9f89cb1..81b4e04 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -544,6 +544,12 @@ config SPI_RSPI
> help
> SPI driver for Renesas RSPI and QSPI blocks.
>
> +config SPI_RENESAS_RPC_IF
Since the driver is called without -IF suffix, I wouldn't use it in the
Kconfig name either, the following is enough:
> + tristate "Renesas R-Car Gen3 RPC-IF SPI controller"
> + depends on ARCH_RENESAS || COMPILE_TEST
Judging on the compilation error reported by the 0-day bot about readq(),
we now need to depend on 64BIT or something...
[...]
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index f296270..3f2b2f9 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
[...]
> diff --git a/drivers/spi/spi-renesas-rpc.c b/drivers/spi/spi-renesas-rpc.c
> new file mode 100644
> index 000..1e57eb1
> --- /dev/null
> +++ b/drivers/spi/spi-renesas-rpc.c
> @@ -0,0 +1,787 @@
[...]
> +#define RPC_CMNCR0x // R/W
> +#define RPC_CMNCR_MD BIT(31)
> +#define RPC_CMNCR_SFDE BIT(24) // undocumented bit but must be
> set
> +#define RPC_CMNCR_MOIIO3(val)(((val) & 0x3) << 22)
> +#define RPC_CMNCR_MOIIO2(val)(((val) & 0x3) << 20)
> +#define RPC_CMNCR_MOIIO1(val)(((val) & 0x3) << 18)
> +#define RPC_CMNCR_MOIIO0(val)(((val) & 0x3) << 16)
> +#define RPC_CMNCR_MOIIO_HIZ (RPC_CMNCR_MOIIO0(3) | RPC_CMNCR_MOIIO1(3) | \
> + RPC_CMNCR_MOIIO2(3) | RPC_CMNCR_MOIIO3(3))
> +#define RPC_CMNCR_IO3FV(val) (((val) & 0x3) << 14) // undocumented bit
> +#define RPC_CMNCR_IO2FV(val) (((val) & 0x3) << 12) // undocumented bit
Those are not exactly bits. I'd be happy with just // undocumented...
[...]
> +#define RPC_WBUF 0x8000 // Write Buffer
> +#define RPC_WBUF_SIZE256 // Write Buffer size
I wonder if the write buffer should be in a separate "reg" prop tuple...
Have you considered that?
[...]
> +static void rpc_spi_hw_init(struct rpc_spi *rpc)
> +{
> + //
> + // NOTE: The 0x260 are undocumented bits, but they must be set.
> + // RPC_PHYCNT_STRTIM is strobe timing adjustment bit,
> + // 0x0 : the delay is biggest,
> + // 0x1 : the delay is 2nd biggest,
> + // On H3 ES1.x, the value should be 0, while on others,
> + // the value should be 6.
> + //
> + regmap_write(rpc->regmap, RPC_PHYCNT, RPC_PHYCNT_CAL |
> + RPC_PHYCNT_STRTIM(6) | 0x260);
> +
> + //
> + // NOTE: The 0x1511144 are undocumented bits, but they must be set
> + // for RPC_PHYOFFSET1.
> + // The 0x31 are undocumented bits, but they must be set
> + // for RPC_PHYOFFSET2.
> + //
> + regmap_write(rpc->regmap, RPC_PHYOFFSET1,
> + RPC_PHYOFFSET1_DDRTMG(3) | 0x1511144);
> + regmap_write(rpc->regmap, RPC_PHYOFFSET2, 0x31 |
> + RPC_PHYOFFSET2_OCTTMG(4));
I still would have preferred using regmap_update_bits() here...
[...]
> +static int rpc_spi_io_xfer(struct rpc_spi *rpc,
> +const void *tx_buf, void *rx_buf)
> +{
[...]
> + } else if (rx_buf) {
> + //
> + // RPC-IF spoils the data for the commands without an address
> + // phase (like RDID) in the manual mode, so we'll have to work
> + // around this issue by using the external address space read
> + // mode instead; we seem to be able to read 8 bytes at most in
> + // this mode though...
> + //
> + if (!(smenr & RPC_SMENR_ADE(0xf))) {
> + u32 nbytes = rpc->xferlen - pos;
> + u64 tmp;
> +
> + if (nbytes > 8)
> + nbytes = 8;
> +
> + regmap_update_bits(rpc->regmap, RPC_CMNCR,
> +RPC_CMNCR_MD, 0);
> + regmap_write(rpc->regmap, RPC_DRCR, 0);
> + regmap_write(rpc->regmap, RPC_DREAR, RPC_DREAR_EAC(1));
> + regmap_write(rpc->regmap, RPC_DRCMR, rpc->cmd);
> + regmap_write(rpc->regmap, RPC_DRDMCR, rpc->dummy);
> + regmap_write(rpc->regmap, RPC_DROPR, 0);
> + regmap_write(rpc->regmap, RPC_DRENR, rpc->smenr &
> + ~RPC_SMENR_SPIDE(0xf));
The 'smenr'
Re: [PATCH v5 1/2] spi: Add Renesas R-Car Gen3 RPC-IF SPI controller driver
Hi Mason,
On Tue, Jan 8, 2019 at 5:17 AM Mason Yang wrote:
> Add a driver for Renesas R-Car Gen3 RPC-IF SPI controller.
>
> Signed-off-by: Mason Yang
Thanks for your patch!
> --- /dev/null
> +++ b/drivers/spi/spi-renesas-rpc.c
> +static void rpc_spi_mem_set_prep_op_cfg(struct spi_device *spi,
> + const struct spi_mem_op *op,
> + u64 *offs, size_t *len)
> +{
> + struct rpc_spi *rpc = spi_master_get_devdata(spi->master);
spi_controller_get_devdata(), for new code.
> +static ssize_t rpc_spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
> + u64 offs, size_t len, void *buf)
> +{
> + struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
> + int ret;
> +
> + if (WARN_ON(offs + desc->info.offset + len > U32_MAX))
> + return -EINVAL;
Why do you need a WARN_ON()?
> +
> + if (WARN_ON(len > 0x400))
> + len = 0x400;
Please drop the WARN_ON(), as this is normal behavior, cfr.:
* @dirmap_write: write data to the memory device using the direct mapping
*created by ->dirmap_create(). The function can return less
*data than requested (for example when the request is crossing
*the currently mapped area), and the caller of
*spi_mem_dirmap_write() is responsible for calling it again in
*this case.
> +static ssize_t rpc_spi_mem_dirmap_write(struct spi_mem_dirmap_desc *desc,
> + u64 offs, size_t len, const void *buf)
> +{
> + struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
> + int ret;
> +
> + if (WARN_ON(offs + desc->info.offset + len > U32_MAX))
> + return -EINVAL;
Why do you need a WARN_ON()?
> +
> + if (WARN_ON(len > RPC_WBUF_SIZE))
> + len = RPC_WBUF_SIZE;
Please drop the WARN_ON().
> +static int rpc_spi_transfer_one_message(struct spi_master *master,
> + struct spi_message *msg)
> +{
> + struct rpc_spi *rpc = spi_master_get_devdata(master);
> + struct spi_transfer *t;
> + int ret;
> +
> + rpc_spi_transfer_setup(rpc, msg);
> +
> + list_for_each_entry(t, &msg->transfers, transfer_list) {
> + if (!list_is_last(&t->transfer_list, &msg->transfers))
> + continue;
Can't you use list_last_entry(), to avoid having to loop over the whole list?
> + ret = rpc_spi_xfer_message(rpc, t);
> + if (ret)
> + goto out;
> + }
> +
> + msg->status = 0;
> + msg->actual_length = rpc->totalxferlen;
> +out:
> + spi_finalize_current_message(master);
> + return 0;
> +}
> +static int rpc_spi_probe(struct platform_device *pdev)
> +{
> + struct spi_master *master;
struct spi_controller, for new code.
> + struct resource *res;
> + struct rpc_spi *rpc;
> + const struct regmap_config *regmap_config;
> + const char *mode;
> + int ret;
> +
> + ret = of_property_read_string(pdev->dev.of_node,
> + "renesas,rpc-mode", &mode);
> + if (ret < 0)
> + return ret;
> +
> + if (strcasecmp(mode, "spi"))
> + return -ENODEV;
> +
> + master = spi_alloc_master(&pdev->dev, sizeof(*rpc));
> + if (!master)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, master);
> +
> + rpc = spi_master_get_devdata(master);
> +
> + master->dev.of_node = pdev->dev.of_node;
> +
> + rpc->clk_rpc = devm_clk_get(&pdev->dev, "rpc");
> + if (IS_ERR(rpc->clk_rpc))
> + return PTR_ERR(rpc->clk_rpc);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> + rpc->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(rpc->base))
> + return PTR_ERR(rpc->base);
> +
> + regmap_config = &rpc_spi_regmap_config;
> + rpc->regmap = devm_regmap_init_mmio(&pdev->dev, rpc->base,
> + regmap_config);
> + if (IS_ERR(rpc->regmap)) {
> + dev_err(&pdev->dev, "failed to init regmap %ld for rpc-spi\n",
> + PTR_ERR(rpc->regmap));
> + return PTR_ERR(rpc->regmap);
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dirmap");
> + rpc->dirmap = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(rpc->dirmap))
> + rpc->dirmap = NULL;
> +
> + rpc->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> + if (IS_ERR(rpc->rstc))
> + return PTR_ERR(rpc->rstc);
> +
> + pm_runtime_enable(&pdev->dev);
> + master->auto_runtime_pm = true;
> +
> + master->num_chipselect = 1;
> +
Re: [PATCH v5 1/2] spi: Add Renesas R-Car Gen3 RPC-IF SPI controller driver
Hi Mason,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on spi/for-next]
[also build test ERROR on v5.0-rc1 next-20190108]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Mason-Yang/spi-Add-Renesas-R-Car-Gen3-RPC-IF-SPI-controller-driver/20190108-165354
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: nds32-allyesconfig (attached as .config)
compiler: nds32le-linux-gcc (GCC) 6.4.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=6.4.0 make.cross ARCH=nds32
All errors (new ones prefixed by >>):
drivers/spi/spi-renesas-rpc.c: In function 'rpc_spi_io_xfer':
>> drivers/spi/spi-renesas-rpc.c:285:10: error: implicit declaration of
>> function 'readq' [-Werror=implicit-function-declaration]
tmp = readq(rpc->dirmap);
^
cc1: some warnings being treated as errors
vim +/readq +285 drivers/spi/spi-renesas-rpc.c
222
223 static int rpc_spi_io_xfer(struct rpc_spi *rpc,
224 const void *tx_buf, void *rx_buf)
225 {
226 u32 smenr, smcr, data, pos = 0;
227 int ret = 0;
228
229 regmap_update_bits(rpc->regmap, RPC_CMNCR, RPC_CMNCR_MD,
RPC_CMNCR_MD);
230 regmap_write(rpc->regmap, RPC_SMDRENR, 0);
231 regmap_write(rpc->regmap, RPC_SMCMR, rpc->cmd);
232 regmap_write(rpc->regmap, RPC_SMDMCR, rpc->dummy);
233 regmap_write(rpc->regmap, RPC_SMADR, rpc->addr);
234 smenr = rpc->smenr;
235
236 if (tx_buf) {
237 while (pos < rpc->xferlen) {
238 u32 nbytes = rpc->xferlen - pos;
239
240 regmap_write(rpc->regmap, RPC_SMWDR0,
241 get_unaligned((u32 *)(tx_buf +
pos)));
242
243 smcr = rpc->smcr | RPC_SMCR_SPIE;
244
245 if (nbytes > 4) {
246 nbytes = 4;
247 smcr |= RPC_SMCR_SSLKP;
248 }
249
250 regmap_write(rpc->regmap, RPC_SMENR, smenr);
251 regmap_write(rpc->regmap, RPC_SMCR, smcr);
252 ret = wait_msg_xfer_end(rpc);
253 if (ret)
254 goto out;
255
256 pos += nbytes;
257 smenr = rpc->smenr & ~RPC_SMENR_CDE &
258 ~RPC_SMENR_ADE(0xf);
259 }
260 } else if (rx_buf) {
261 //
262 // RPC-IF spoils the data for the commands without an
address
263 // phase (like RDID) in the manual mode, so we'll have
to work
264 // around this issue by using the external address
space read
265 // mode instead; we seem to be able to read 8 bytes at
most in
266 // this mode though...
267 //
268 if (!(smenr & RPC_SMENR_ADE(0xf))) {
269 u32 nbytes = rpc->xferlen - pos;
270 u64 tmp;
271
272 if (nbytes > 8)
273 nbytes = 8;
274
275 regmap_update_bits(rpc->regmap, RPC_CMNCR,
276 RPC_CMNCR_MD, 0);
277 regmap_write(rpc->regmap, RPC_DRCR, 0);
278 regmap_write(rpc->regmap, RPC_DREAR,
RPC_DREAR_EAC(1));
279 regmap_write(rpc->regmap, RPC_DRCMR, rpc->cmd);
280 regmap_write(rpc->regmap, RPC_DRDMCR,
rpc->dummy);
281 regmap_write(rpc->regmap, RPC_DROPR, 0);
282 regmap_write(rpc->regmap, RPC_DRENR, rpc->smenr
&
283 ~RPC_SMENR_SPIDE(0xf));
284
> 285 tmp = readq(rpc->dirmap);
286 memcpy(rx_buf, &tmp, nbytes);
287 } else {
288 while (pos < rpc->xferlen) {
289 u32 nbytes = rpc->xferlen - pos;
290
291 if (nbytes > 4)
292 nbytes = 4;
293
294 regmap_write(rpc->regmap, RPC_SMENR,
smenr);
295 regmap_write(rpc->reg
