linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
@ 2022-09-26 14:29 Yang Yingliang
  2022-09-26 14:29 ` [PATCH -next 2/2] spi: xilinx: switch to use spi_controller_*() functions Yang Yingliang
  2022-09-27  3:45 ` [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller() Lukas Wunner
  0 siblings, 2 replies; 19+ messages in thread
From: Yang Yingliang @ 2022-09-26 14:29 UTC (permalink / raw)
  To: linux-spi; +Cc: broonie, lukas, yangyingliang

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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH -next 2/2] spi: xilinx: switch to use spi_controller_*() functions
  2022-09-26 14:29 [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller() Yang Yingliang
@ 2022-09-26 14:29 ` Yang Yingliang
  2022-09-27  3:45 ` [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller() Lukas Wunner
  1 sibling, 0 replies; 19+ messages in thread
From: Yang Yingliang @ 2022-09-26 14:29 UTC (permalink / raw)
  To: linux-spi; +Cc: broonie, lukas, yangyingliang

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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-26 14:29 [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller() Yang Yingliang
  2022-09-26 14:29 ` [PATCH -next 2/2] spi: xilinx: switch to use spi_controller_*() functions Yang Yingliang
@ 2022-09-27  3:45 ` Lukas Wunner
  2022-09-27  7:11   ` Geert Uytterhoeven
  2022-09-27 11:21   ` Mark Brown
  1 sibling, 2 replies; 19+ messages in thread
From: Lukas Wunner @ 2022-09-27  3:45 UTC (permalink / raw)
  To: Yang Yingliang; +Cc: linux-spi, broonie, Geert Uytterhoeven

[+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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-27  3:45 ` [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller() Lukas Wunner
@ 2022-09-27  7:11   ` Geert Uytterhoeven
  2022-09-27 11:21   ` Mark Brown
  1 sibling, 0 replies; 19+ messages in thread
From: Geert Uytterhoeven @ 2022-09-27  7:11 UTC (permalink / raw)
  To: Lukas Wunner; +Cc: Yang Yingliang, linux-spi, broonie

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-27  3:45 ` [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller() Lukas Wunner
  2022-09-27  7:11   ` Geert Uytterhoeven
@ 2022-09-27 11:21   ` Mark Brown
  2022-09-27 11:57     ` Yang Yingliang
  1 sibling, 1 reply; 19+ messages in thread
From: Mark Brown @ 2022-09-27 11:21 UTC (permalink / raw)
  To: Lukas Wunner; +Cc: Yang Yingliang, linux-spi, Geert Uytterhoeven

[-- 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 --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-27 11:21   ` Mark Brown
@ 2022-09-27 11:57     ` Yang Yingliang
  2022-09-27 13:31       ` Lukas Wunner
  0 siblings, 1 reply; 19+ messages in thread
From: Yang Yingliang @ 2022-09-27 11:57 UTC (permalink / raw)
  To: Mark Brown, Lukas Wunner; +Cc: linux-spi, Geert Uytterhoeven, yangyingliang

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-27 11:57     ` Yang Yingliang
@ 2022-09-27 13:31       ` Lukas Wunner
  2022-09-27 17:01         ` Mark Brown
  0 siblings, 1 reply; 19+ messages in thread
From: Lukas Wunner @ 2022-09-27 13:31 UTC (permalink / raw)
  To: Yang Yingliang; +Cc: Mark Brown, linux-spi, Geert Uytterhoeven

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-27 13:31       ` Lukas Wunner
@ 2022-09-27 17:01         ` Mark Brown
  2022-09-27 20:19           ` Lukas Wunner
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2022-09-27 17:01 UTC (permalink / raw)
  To: Lukas Wunner; +Cc: Yang Yingliang, linux-spi, Geert Uytterhoeven

[-- 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 --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-27 17:01         ` Mark Brown
@ 2022-09-27 20:19           ` Lukas Wunner
  2022-09-27 20:22             ` Mark Brown
  0 siblings, 1 reply; 19+ messages in thread
From: Lukas Wunner @ 2022-09-27 20:19 UTC (permalink / raw)
  To: Mark Brown; +Cc: Yang Yingliang, linux-spi, Geert Uytterhoeven

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-27 20:19           ` Lukas Wunner
@ 2022-09-27 20:22             ` Mark Brown
  2022-09-27 20:43               ` Geert Uytterhoeven
  2022-09-28  6:34               ` Yang Yingliang
  0 siblings, 2 replies; 19+ messages in thread
From: Mark Brown @ 2022-09-27 20:22 UTC (permalink / raw)
  To: Lukas Wunner; +Cc: Yang Yingliang, linux-spi, Geert Uytterhoeven

[-- 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 --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-27 20:22             ` Mark Brown
@ 2022-09-27 20:43               ` Geert Uytterhoeven
  2022-09-28 11:15                 ` Mark Brown
  2022-09-28  6:34               ` Yang Yingliang
  1 sibling, 1 reply; 19+ messages in thread
From: Geert Uytterhoeven @ 2022-09-27 20:43 UTC (permalink / raw)
  To: Mark Brown; +Cc: Lukas Wunner, Yang Yingliang, linux-spi

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-27 20:22             ` Mark Brown
  2022-09-27 20:43               ` Geert Uytterhoeven
@ 2022-09-28  6:34               ` Yang Yingliang
  2022-09-28 11:10                 ` Mark Brown
  1 sibling, 1 reply; 19+ messages in thread
From: Yang Yingliang @ 2022-09-28  6:34 UTC (permalink / raw)
  To: Mark Brown, Lukas Wunner; +Cc: linux-spi, Geert Uytterhoeven, yangyingliang


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-28  6:34               ` Yang Yingliang
@ 2022-09-28 11:10                 ` Mark Brown
  0 siblings, 0 replies; 19+ messages in thread
From: Mark Brown @ 2022-09-28 11:10 UTC (permalink / raw)
  To: Yang Yingliang; +Cc: Lukas Wunner, linux-spi, Geert Uytterhoeven

[-- 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 --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-27 20:43               ` Geert Uytterhoeven
@ 2022-09-28 11:15                 ` Mark Brown
  2022-09-28 15:01                   ` Yang Yingliang
  2022-09-28 15:11                   ` Lukas Wunner
  0 siblings, 2 replies; 19+ messages in thread
From: Mark Brown @ 2022-09-28 11:15 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Lukas Wunner, Yang Yingliang, linux-spi

[-- 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 --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-28 11:15                 ` Mark Brown
@ 2022-09-28 15:01                   ` Yang Yingliang
  2022-09-28 16:08                     ` Mark Brown
  2022-09-28 15:11                   ` Lukas Wunner
  1 sibling, 1 reply; 19+ messages in thread
From: Yang Yingliang @ 2022-09-28 15:01 UTC (permalink / raw)
  To: Mark Brown, Geert Uytterhoeven; +Cc: Lukas Wunner, linux-spi, yangyingliang


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-28 11:15                 ` Mark Brown
  2022-09-28 15:01                   ` Yang Yingliang
@ 2022-09-28 15:11                   ` Lukas Wunner
  2022-09-28 15:58                     ` Mark Brown
  2022-09-28 17:14                     ` Geert Uytterhoeven
  1 sibling, 2 replies; 19+ messages in thread
From: Lukas Wunner @ 2022-09-28 15:11 UTC (permalink / raw)
  To: Mark Brown; +Cc: Geert Uytterhoeven, Yang Yingliang, linux-spi

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-28 15:11                   ` Lukas Wunner
@ 2022-09-28 15:58                     ` Mark Brown
  2022-09-28 17:14                     ` Geert Uytterhoeven
  1 sibling, 0 replies; 19+ messages in thread
From: Mark Brown @ 2022-09-28 15:58 UTC (permalink / raw)
  To: Lukas Wunner; +Cc: Geert Uytterhoeven, Yang Yingliang, linux-spi

[-- 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 --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-28 15:01                   ` Yang Yingliang
@ 2022-09-28 16:08                     ` Mark Brown
  0 siblings, 0 replies; 19+ messages in thread
From: Mark Brown @ 2022-09-28 16:08 UTC (permalink / raw)
  To: Yang Yingliang; +Cc: Geert Uytterhoeven, Lukas Wunner, linux-spi

[-- 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 --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller()
  2022-09-28 15:11                   ` Lukas Wunner
  2022-09-28 15:58                     ` Mark Brown
@ 2022-09-28 17:14                     ` Geert Uytterhoeven
  1 sibling, 0 replies; 19+ messages in thread
From: Geert Uytterhoeven @ 2022-09-28 17:14 UTC (permalink / raw)
  To: Lukas Wunner; +Cc: Mark Brown, Yang Yingliang, linux-spi

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-09-28 17:15 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-26 14:29 [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller() Yang Yingliang
2022-09-26 14:29 ` [PATCH -next 2/2] spi: xilinx: switch to use spi_controller_*() functions Yang Yingliang
2022-09-27  3:45 ` [PATCH -next 1/2] spi: introduce devm_spi_alloc_controller() Lukas Wunner
2022-09-27  7:11   ` Geert Uytterhoeven
2022-09-27 11:21   ` Mark Brown
2022-09-27 11:57     ` Yang Yingliang
2022-09-27 13:31       ` Lukas Wunner
2022-09-27 17:01         ` Mark Brown
2022-09-27 20:19           ` Lukas Wunner
2022-09-27 20:22             ` Mark Brown
2022-09-27 20:43               ` Geert Uytterhoeven
2022-09-28 11:15                 ` Mark Brown
2022-09-28 15:01                   ` Yang Yingliang
2022-09-28 16:08                     ` Mark Brown
2022-09-28 15:11                   ` Lukas Wunner
2022-09-28 15:58                     ` Mark Brown
2022-09-28 17:14                     ` Geert Uytterhoeven
2022-09-28  6:34               ` Yang Yingliang
2022-09-28 11:10                 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).