[PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()

[PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()

[Yang Yingliang]

Introduce devm_spi_alloc_controller() to wrap __devm_spi_alloc_controller(),
with this wrapper, the drivers can use it to update to the modern naming.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 include/linux/spi/spi.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 6ea889df0813..32dd736ebe2e 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -778,6 +778,12 @@ static inline struct spi_controller *devm_spi_alloc_slave(struct device *dev,
 	return __devm_spi_alloc_controller(dev, size, true);
 }
 
+static inline struct spi_controller *devm_spi_alloc_controller(struct device *dev,
+							       unsigned int size)
+{
+	return __devm_spi_alloc_controller(dev, size, false);
+}
+
 extern int spi_register_controller(struct spi_controller *ctlr);
 extern int devm_spi_register_controller(struct device *dev,
 					struct spi_controller *ctlr);
-- 
2.25.1

[PATCH -next 2/2] spi: xilinx: switch to use spi_controller_*() functions

[Yang Yingliang]

Switch to use devm_spi_alloc_controller(), spi_controller_get_devdata() and
spi_controller_put(), it's no functional changed.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/spi/spi-xilinx.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/spi/spi-xilinx.c b/drivers/spi/spi-xilinx.c
index 7377d3b81302..307872d7b5a5 100644
--- a/drivers/spi/spi-xilinx.c
+++ b/drivers/spi/spi-xilinx.c
@@ -185,7 +185,7 @@ static void xspi_init_hw(struct xilinx_spi *xspi)
 
 static void xilinx_spi_chipselect(struct spi_device *spi, int is_on)
 {
-	struct xilinx_spi *xspi = spi_master_get_devdata(spi->master);
+	struct xilinx_spi *xspi = spi_controller_get_devdata(spi->controller);
 	u16 cr;
 	u32 cs;
 
@@ -225,7 +225,7 @@ static void xilinx_spi_chipselect(struct spi_device *spi, int is_on)
 static int xilinx_spi_setup_transfer(struct spi_device *spi,
 		struct spi_transfer *t)
 {
-	struct xilinx_spi *xspi = spi_master_get_devdata(spi->master);
+	struct xilinx_spi *xspi = spi_controller_get_devdata(spi->controller);
 
 	if (spi->mode & SPI_CS_HIGH)
 		xspi->cs_inactive &= ~BIT(spi->chip_select);
@@ -237,7 +237,7 @@ static int xilinx_spi_setup_transfer(struct spi_device *spi,
 
 static int xilinx_spi_txrx_bufs(struct spi_device *spi, struct spi_transfer *t)
 {
-	struct xilinx_spi *xspi = spi_master_get_devdata(spi->master);
+	struct xilinx_spi *xspi = spi_controller_get_devdata(spi->controller);
 	int remaining_words;	/* the number of words left to transfer */
 	bool use_irq = false;
 	u16 cr = 0;
@@ -392,7 +392,7 @@ static int xilinx_spi_probe(struct platform_device *pdev)
 	struct xspi_platform_data *pdata;
 	struct resource *res;
 	int ret, num_cs = 0, bits_per_word;
-	struct spi_master *master;
+	struct spi_controller *ctlr;
 	u32 tmp;
 	u8 i;
 
@@ -421,17 +421,17 @@ static int xilinx_spi_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	master = devm_spi_alloc_master(&pdev->dev, sizeof(struct xilinx_spi));
-	if (!master)
+	ctlr = devm_spi_alloc_controller(&pdev->dev, sizeof(struct xilinx_spi));
+	if (!ctlr)
 		return -ENODEV;
 
 	/* the spi->mode bits understood by this driver: */
-	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LSB_FIRST | SPI_LOOP |
-			    SPI_CS_HIGH;
+	ctlr->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LSB_FIRST | SPI_LOOP |
+			  SPI_CS_HIGH;
 
-	xspi = spi_master_get_devdata(master);
+	xspi = spi_controller_get_devdata(ctlr);
 	xspi->cs_inactive = 0xffffffff;
-	xspi->bitbang.master = master;
+	xspi->bitbang.master = ctlr;
 	xspi->bitbang.chipselect = xilinx_spi_chipselect;
 	xspi->bitbang.setup_transfer = xilinx_spi_setup_transfer;
 	xspi->bitbang.txrx_bufs = xilinx_spi_txrx_bufs;
@@ -442,9 +442,9 @@ static int xilinx_spi_probe(struct platform_device *pdev)
 	if (IS_ERR(xspi->regs))
 		return PTR_ERR(xspi->regs);
 
-	master->bus_num = pdev->id;
-	master->num_chipselect = num_cs;
-	master->dev.of_node = pdev->dev.of_node;
+	ctlr->bus_num = pdev->id;
+	ctlr->num_chipselect = num_cs;
+	ctlr->dev.of_node = pdev->dev.of_node;
 
 	/*
 	 * Detect endianess on the IP via loop bit in CR. Detection
@@ -464,7 +464,7 @@ static int xilinx_spi_probe(struct platform_device *pdev)
 		xspi->write_fn = xspi_write32_be;
 	}
 
-	master->bits_per_word_mask = SPI_BPW_MASK(bits_per_word);
+	ctlr->bits_per_word_mask = SPI_BPW_MASK(bits_per_word);
 	xspi->bytes_per_word = bits_per_word / 8;
 	xspi->buffer_size = xilinx_spi_find_buffer_size(xspi);
 
@@ -492,17 +492,17 @@ static int xilinx_spi_probe(struct platform_device *pdev)
 
 	if (pdata) {
 		for (i = 0; i < pdata->num_devices; i++)
-			spi_new_device(master, pdata->devices + i);
+			spi_new_device(ctlr, pdata->devices + i);
 	}
 
-	platform_set_drvdata(pdev, master);
+	platform_set_drvdata(pdev, ctlr);
 	return 0;
 }
 
 static int xilinx_spi_remove(struct platform_device *pdev)
 {
-	struct spi_master *master = platform_get_drvdata(pdev);
-	struct xilinx_spi *xspi = spi_master_get_devdata(master);
+	struct spi_controller *ctlr = platform_get_drvdata(pdev);
+	struct xilinx_spi *xspi = spi_controller_get_devdata(ctlr);
 	void __iomem *regs_base = xspi->regs;
 
 	spi_bitbang_stop(&xspi->bitbang);
@@ -512,7 +512,7 @@ static int xilinx_spi_remove(struct platform_device *pdev)
 	/* Disable the global IPIF interrupt */
 	xspi->write_fn(0, regs_base + XIPIF_V123B_DGIER_OFFSET);
 
-	spi_master_put(xspi->bitbang.master);
+	spi_controller_put(xspi->bitbang.master);
 
 	return 0;
 }
-- 
2.25.1

Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()

[Lukas Wunner]

[+cc Geert, who originally came up with "spi_controller"]

On Mon, Sep 26, 2022 at 10:29:32PM +0800, Yang Yingliang wrote:
> Introduce devm_spi_alloc_controller() to wrap __devm_spi_alloc_controller(),
> with this wrapper, the drivers can use it to update to the modern naming.
[...]
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -778,6 +778,12 @@ static inline struct spi_controller *devm_spi_alloc_slave(struct device *dev,
>  	return __devm_spi_alloc_controller(dev, size, true);
>  }
>  
> +static inline struct spi_controller *devm_spi_alloc_controller(struct device *dev,
> +							       unsigned int size)
> +{
> +	return __devm_spi_alloc_controller(dev, size, false);
> +}
> +
>  extern int spi_register_controller(struct spi_controller *ctlr);
>  extern int devm_spi_register_controller(struct device *dev,
>  					struct spi_controller *ctlr);

This doesn't really make sense I'm afraid.  The umbrella term
"spi_controller" can refer to either a master or a slave.
One has to specify on allocation which of the two is desired.

An API which purports to allow allocation of the umbrella term
but defaults to a master behind the scenes seems misleading to me.

Thanks,

Lukas

Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()

[Geert Uytterhoeven]

Hi Lukas,

On Tue, Sep 27, 2022 at 5:45 AM Lukas Wunner <lukas@wunner.de> wrote:
> [+cc Geert, who originally came up with "spi_controller"]
>
> On Mon, Sep 26, 2022 at 10:29:32PM +0800, Yang Yingliang wrote:
> > Introduce devm_spi_alloc_controller() to wrap __devm_spi_alloc_controller(),
> > with this wrapper, the drivers can use it to update to the modern naming.
> [...]
> > --- a/include/linux/spi/spi.h
> > +++ b/include/linux/spi/spi.h
> > @@ -778,6 +778,12 @@ static inline struct spi_controller *devm_spi_alloc_slave(struct device *dev,
> >       return __devm_spi_alloc_controller(dev, size, true);
> >  }
> >
> > +static inline struct spi_controller *devm_spi_alloc_controller(struct device *dev,
> > +                                                            unsigned int size)
> > +{
> > +     return __devm_spi_alloc_controller(dev, size, false);
> > +}
> > +
> >  extern int spi_register_controller(struct spi_controller *ctlr);
> >  extern int devm_spi_register_controller(struct device *dev,
> >                                       struct spi_controller *ctlr);
>
> This doesn't really make sense I'm afraid.  The umbrella term
> "spi_controller" can refer to either a master or a slave.
> One has to specify on allocation which of the two is desired.
>
> An API which purports to allow allocation of the umbrella term
> but defaults to a master behind the scenes seems misleading to me.

Moreover, you already added devm_spi_alloc_master() and
devm_spi_alloc_slave() in commit 5e844cc37a5cbaa4 ("spi: Introduce
device-managed SPI controller allocation") in v5.10 ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 -next 1/2] spi: introduce devm_spi_alloc_controller()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

On Tue, Sep 27, 2022 at 05:45:25AM +0200, Lukas Wunner wrote:
> On Mon, Sep 26, 2022 at 10:29:32PM +0800, Yang Yingliang wrote:

> >  extern int devm_spi_register_controller(struct device *dev,
> >  					struct spi_controller *ctlr);

> This doesn't really make sense I'm afraid.  The umbrella term
> "spi_controller" can refer to either a master or a slave.
> One has to specify on allocation which of the two is desired.

> An API which purports to allow allocation of the umbrella term
> but defaults to a master behind the scenes seems misleading to me.

Yes, we'd need to either have two wrappers using some more appropriate
terminology than master/slave or have a parameter which specifies the
role.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()

[Yang Yingliang]

Hi Mark,

On 2022/9/27 19:21, Mark Brown wrote:
> On Tue, Sep 27, 2022 at 05:45:25AM +0200, Lukas Wunner wrote:
>> On Mon, Sep 26, 2022 at 10:29:32PM +0800, Yang Yingliang wrote:
>>>   extern int devm_spi_register_controller(struct device *dev,
>>>   					struct spi_controller *ctlr);
>> This doesn't really make sense I'm afraid.  The umbrella term
>> "spi_controller" can refer to either a master or a slave.
>> One has to specify on allocation which of the two is desired.
>> An API which purports to allow allocation of the umbrella term
>> but defaults to a master behind the scenes seems misleading to me.
> Yes, we'd need to either have two wrappers using some more appropriate
> terminology than master/slave or have a parameter which specifies the
> role.
Do you mean to introduce two more proper wrappers to instead of 
devm_spi_alloc_master/slave() ?

Thanks,
Yang

Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()

[Lukas Wunner]

On Tue, Sep 27, 2022 at 07:57:05PM +0800, Yang Yingliang wrote:
> On 2022/9/27 19:21, Mark Brown wrote:
> > On Tue, Sep 27, 2022 at 05:45:25AM +0200, Lukas Wunner wrote:
> > > On Mon, Sep 26, 2022 at 10:29:32PM +0800, Yang Yingliang wrote:
> > > >   extern int devm_spi_register_controller(struct device *dev,
> > > >   					struct spi_controller *ctlr);
> > > This doesn't really make sense I'm afraid.  The umbrella term
> > > "spi_controller" can refer to either a master or a slave.
> > > One has to specify on allocation which of the two is desired.
> > > An API which purports to allow allocation of the umbrella term
> > > but defaults to a master behind the scenes seems misleading to me.
> > Yes, we'd need to either have two wrappers using some more appropriate
> > terminology than master/slave or have a parameter which specifies the
> > role.
> Do you mean to introduce two more proper wrappers to instead of
> devm_spi_alloc_master/slave() ?

Honestly I don't think there's room for (or a need for) improvement here.

Thanks,

Lukas

Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

On Tue, Sep 27, 2022 at 03:31:29PM +0200, Lukas Wunner wrote:
> On Tue, Sep 27, 2022 at 07:57:05PM +0800, Yang Yingliang wrote:

> > Do you mean to introduce two more proper wrappers to instead of
> > devm_spi_alloc_master/slave() ?

> Honestly I don't think there's room for (or a need for) improvement here.

The issue here is that we're trying to get rid of the master/slave
terminology.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()

[Lukas Wunner]

On Tue, Sep 27, 2022 at 06:01:39PM +0100, Mark Brown wrote:
> On Tue, Sep 27, 2022 at 03:31:29PM +0200, Lukas Wunner wrote:
> > On Tue, Sep 27, 2022 at 07:57:05PM +0800, Yang Yingliang wrote:
> > > Do you mean to introduce two more proper wrappers to instead of
> > > devm_spi_alloc_master/slave() ?
> 
> > Honestly I don't think there's room for (or a need for) improvement here.
> 
> The issue here is that we're trying to get rid of the master/slave
> terminology.

Converting drivers to use spi_controller everywhere in lieu of
spi_master is fine, but drivers need to specify whether the
spi_controller is a master or a slave and Geert's design is
to specify that on allocation.  Which makes sense because
that's the moment the spi_controller comes to life, there's
no earlier moment where one could specify the type.

Thanks,

Lukas

Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

On Tue, Sep 27, 2022 at 10:19:01PM +0200, Lukas Wunner wrote:
> On Tue, Sep 27, 2022 at 06:01:39PM +0100, Mark Brown wrote:
> > On Tue, Sep 27, 2022 at 03:31:29PM +0200, Lukas Wunner wrote:
> > > On Tue, Sep 27, 2022 at 07:57:05PM +0800, Yang Yingliang wrote:

> > > > Do you mean to introduce two more proper wrappers to instead of
> > > > devm_spi_alloc_master/slave() ?

> > > Honestly I don't think there's room for (or a need for) improvement here.

> > The issue here is that we're trying to get rid of the master/slave
> > terminology.

> Converting drivers to use spi_controller everywhere in lieu of
> spi_master is fine, but drivers need to specify whether the
> spi_controller is a master or a slave and Geert's design is
> to specify that on allocation.  Which makes sense because
> that's the moment the spi_controller comes to life, there's
> no earlier moment where one could specify the type.

Sure, but since the current wrappers use the legacy names this means
that we need new wrappers with more modern names hence there is
something to improve here.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()

[Geert Uytterhoeven]

Hi Mark,

On Tue, Sep 27, 2022 at 10:24 PM Mark Brown <broonie@kernel.org> wrote:
> On Tue, Sep 27, 2022 at 10:19:01PM +0200, Lukas Wunner wrote:
> > On Tue, Sep 27, 2022 at 06:01:39PM +0100, Mark Brown wrote:
> > > On Tue, Sep 27, 2022 at 03:31:29PM +0200, Lukas Wunner wrote:
> > > > On Tue, Sep 27, 2022 at 07:57:05PM +0800, Yang Yingliang wrote:
>
> > > > > Do you mean to introduce two more proper wrappers to instead of
> > > > > devm_spi_alloc_master/slave() ?
>
> > > > Honestly I don't think there's room for (or a need for) improvement here.
>
> > > The issue here is that we're trying to get rid of the master/slave
> > > terminology.
>
> > Converting drivers to use spi_controller everywhere in lieu of
> > spi_master is fine, but drivers need to specify whether the
> > spi_controller is a master or a slave and Geert's design is
> > to specify that on allocation.  Which makes sense because
> > that's the moment the spi_controller comes to life, there's
> > no earlier moment where one could specify the type.
>
> Sure, but since the current wrappers use the legacy names this means
> that we need new wrappers with more modern names hence there is
> something to improve here.

So what are the more modern names?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 -next 1/2] spi: introduce devm_spi_alloc_controller()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]

On Tue, Sep 27, 2022 at 10:43:08PM +0200, Geert Uytterhoeven wrote:
> On Tue, Sep 27, 2022 at 10:24 PM Mark Brown <broonie@kernel.org> wrote:

> > Sure, but since the current wrappers use the legacy names this means
> > that we need new wrappers with more modern names hence there is
> > something to improve here.

> So what are the more modern names?

It's unfortunately not 100% clear, and our use of controller for the
generic thing gets in the way a bit.  There was some stuff from one of
the open source hardware groups recently that tried to propose new names
but I'm not immediately finding it.  "host" and "target" would probably
do the trick?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()

[Yang Yingliang]

On 2022/9/28 19:15, Mark Brown wrote:
> On Tue, Sep 27, 2022 at 10:43:08PM +0200, Geert Uytterhoeven wrote:
>> On Tue, Sep 27, 2022 at 10:24 PM Mark Brown <broonie@kernel.org> wrote:
>>> Sure, but since the current wrappers use the legacy names this means
>>> that we need new wrappers with more modern names hence there is
>>> something to improve here.
>> So what are the more modern names?
> It's unfortunately not 100% clear, and our use of controller for the
> generic thing gets in the way a bit.  There was some stuff from one of
> the open source hardware groups recently that tried to propose new names
> but I'm not immediately finding it.  "host" and "target" would probably
> do the trick?
So shall we use host/target to do wrappers.

Thanks,
Yang

Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 514 bytes --]

On Wed, Sep 28, 2022 at 11:01:00PM +0800, Yang Yingliang wrote:
> On 2022/9/28 19:15, Mark Brown wrote:

> > It's unfortunately not 100% clear, and our use of controller for the
> > generic thing gets in the way a bit.  There was some stuff from one of
> > the open source hardware groups recently that tried to propose new names
> > but I'm not immediately finding it.  "host" and "target" would probably
> > do the trick?

> So shall we use host/target to do wrappers.

Yes, let's try that and see how it looks.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()

[Lukas Wunner]

On Wed, Sep 28, 2022 at 12:15:15PM +0100, Mark Brown wrote:
> On Tue, Sep 27, 2022 at 10:43:08PM +0200, Geert Uytterhoeven wrote:
> > On Tue, Sep 27, 2022 at 10:24 PM Mark Brown <broonie@kernel.org> wrote:
> 
> > > Sure, but since the current wrappers use the legacy names this means
> > > that we need new wrappers with more modern names hence there is
> > > something to improve here.
> 
> > So what are the more modern names?
> 
> It's unfortunately not 100% clear, and our use of controller for the
> generic thing gets in the way a bit.  There was some stuff from one of
> the open source hardware groups recently that tried to propose new names
> but I'm not immediately finding it.  "host" and "target" would probably
> do the trick?

Perhaps you mean that one?

https://www.oshwa.org/a-resolution-to-redefine-spi-signal-names/

Looks like they're replacing master with controller and
slave with peripheral.  Pity, we're using controller as
an umbrella term for either one of them.

Renaming that will lead to an awful lot of churn. :(

Thanks,

Lukas

Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 896 bytes --]

On Wed, Sep 28, 2022 at 05:11:16PM +0200, Lukas Wunner wrote:
> On Wed, Sep 28, 2022 at 12:15:15PM +0100, Mark Brown wrote:

> > It's unfortunately not 100% clear, and our use of controller for the
> > generic thing gets in the way a bit.  There was some stuff from one of
> > the open source hardware groups recently that tried to propose new names
> > but I'm not immediately finding it.  "host" and "target" would probably
> > do the trick?

> Perhaps you mean that one?

> https://www.oshwa.org/a-resolution-to-redefine-spi-signal-names/

> Looks like they're replacing master with controller and
> slave with peripheral.  Pity, we're using controller as
> an umbrella term for either one of them.

> Renaming that will lead to an awful lot of churn. :(

Ah, that's the one - I knew there was some reason I didn't do anything
with it when I saw it.  I'm not sure the churn would be worth it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()

[Geert Uytterhoeven]

On Wed, Sep 28, 2022 at 5:11 PM Lukas Wunner <lukas@wunner.de> wrote:
> On Wed, Sep 28, 2022 at 12:15:15PM +0100, Mark Brown wrote:
> > On Tue, Sep 27, 2022 at 10:43:08PM +0200, Geert Uytterhoeven wrote:
> > > On Tue, Sep 27, 2022 at 10:24 PM Mark Brown <broonie@kernel.org> wrote:
> > > > Sure, but since the current wrappers use the legacy names this means
> > > > that we need new wrappers with more modern names hence there is
> > > > something to improve here.
> >
> > > So what are the more modern names?
> >
> > It's unfortunately not 100% clear, and our use of controller for the
> > generic thing gets in the way a bit.  There was some stuff from one of
> > the open source hardware groups recently that tried to propose new names
> > but I'm not immediately finding it.  "host" and "target" would probably
> > do the trick?
>
> Perhaps you mean that one?
>
> https://www.oshwa.org/a-resolution-to-redefine-spi-signal-names/
>
> Looks like they're replacing master with controller and
> slave with peripheral.

In an inconsistent way: there is no peripheral select, but a chip select.

> Pity, we're using controller as
> an umbrella term for either one of them.

So we have a controller controller, and a peripheral controller ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 -next 1/2] spi: introduce devm_spi_alloc_controller()

[Yang Yingliang]

On 2022/9/28 4:22, Mark Brown wrote:
> On Tue, Sep 27, 2022 at 10:19:01PM +0200, Lukas Wunner wrote:
>> On Tue, Sep 27, 2022 at 06:01:39PM +0100, Mark Brown wrote:
>>> On Tue, Sep 27, 2022 at 03:31:29PM +0200, Lukas Wunner wrote:
>>>> On Tue, Sep 27, 2022 at 07:57:05PM +0800, Yang Yingliang wrote:
>>>>> Do you mean to introduce two more proper wrappers to instead of
>>>>> devm_spi_alloc_master/slave() ?
>>>> Honestly I don't think there's room for (or a need for) improvement here.
>>> The issue here is that we're trying to get rid of the master/slave
>>> terminology.
>> Converting drivers to use spi_controller everywhere in lieu of
>> spi_master is fine, but drivers need to specify whether the
>> spi_controller is a master or a slave and Geert's design is
>> to specify that on allocation.  Which makes sense because
>> that's the moment the spi_controller comes to life, there's
>> no earlier moment where one could specify the type.
> Sure, but since the current wrappers use the legacy names this means
> that we need new wrappers with more modern names hence there is
> something to improve here.

How about using primary/secondary, introduce two wrappers like this:

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h

index 6ea889df0813..c41654fb069b 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -778,6 +778,21 @@ static inline struct spi_controller 
*devm_spi_alloc_slave(struct device *dev,
      return __devm_spi_alloc_controller(dev, size, true);
  }

+static inline struct spi_controller *devm_spi_alloc_primary(struct 
device *dev,
+                                unsigned int size)
+{
+    return __devm_spi_alloc_controller(dev, size, false);
+}
+
+static inline struct spi_controller *devm_spi_alloc_secondary(struct 
device *dev,
+                                  unsigned int size)
+{
+    if (!IS_ENABLED(CONFIG_SPI_SLAVE))
+        return NULL;
+
+    return __devm_spi_alloc_controller(dev, size, true);
+}
+
  extern int spi_register_controller(struct spi_controller *ctlr);
  extern int devm_spi_register_controller(struct device *dev,
                      struct spi_controller *ctlr);

Thanks,
Yang

Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 406 bytes --]

On Wed, Sep 28, 2022 at 02:34:17PM +0800, Yang Yingliang wrote:
> On 2022/9/28 4:22, Mark Brown wrote:

> > Sure, but since the current wrappers use the legacy names this means
> > that we need new wrappers with more modern names hence there is
> > something to improve here.

> How about using primary/secondary, introduce two wrappers like this:

That's not going to be clear enough I think.  

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]