diff mbox series

[1/1] spi: altera: Change to dynamic allocation of spi id

Message ID 20211019002401.24041-1-russell.h.weight@intel.com (mailing list archive)
State Accepted
Commit f09f6dfef8ce7b70a240cf83811e2b1909c3e47b
Headers show
Series [1/1] spi: altera: Change to dynamic allocation of spi id | expand

Commit Message

Russ Weight Oct. 19, 2021, 12:24 a.m. UTC
The spi-altera driver has two flavors: platform and dfl. I'm seeing
a case where I have both device types in the same machine, and they
are conflicting on the SPI ID:

... kernel: couldn't get idr
... kernel: WARNING: CPU: 28 PID: 912 at drivers/spi/spi.c:2920 spi_register_controller.cold+0x84/0xc0a

Both the platform and dfl drivers use the parent's driver ID as the SPI
ID. In the error case, the parent devices are dfl_dev.4 and
subdev_spi_altera.4.auto. When the second spi-master is created, the
failure occurs because the SPI ID of 4 has already been allocated.

Change the ID allocation to dynamic (by initializing bus_num to -1) to
avoid duplicate SPI IDs.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
 drivers/spi/spi-altera-dfl.c      | 2 +-
 drivers/spi/spi-altera-platform.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Russ Weight Oct. 19, 2021, 12:27 a.m. UTC | #1
If there are no concerns with this patch, it will need to have the "cc stable"
tag added for earlier kernels.

I don't think it needs a "Fixes" tag, as the problem has been there since the
driver was introduced.

Thanks,
- Russ

On 10/18/21 5:24 PM, Russ Weight wrote:
> The spi-altera driver has two flavors: platform and dfl. I'm seeing
> a case where I have both device types in the same machine, and they
> are conflicting on the SPI ID:
>
> ... kernel: couldn't get idr
> ... kernel: WARNING: CPU: 28 PID: 912 at drivers/spi/spi.c:2920 spi_register_controller.cold+0x84/0xc0a
>
> Both the platform and dfl drivers use the parent's driver ID as the SPI
> ID. In the error case, the parent devices are dfl_dev.4 and
> subdev_spi_altera.4.auto. When the second spi-master is created, the
> failure occurs because the SPI ID of 4 has already been allocated.
>
> Change the ID allocation to dynamic (by initializing bus_num to -1) to
> avoid duplicate SPI IDs.
>
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> ---
>  drivers/spi/spi-altera-dfl.c      | 2 +-
>  drivers/spi/spi-altera-platform.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/spi-altera-dfl.c b/drivers/spi/spi-altera-dfl.c
> index 44fc9ee13fc7..ca40923258af 100644
> --- a/drivers/spi/spi-altera-dfl.c
> +++ b/drivers/spi/spi-altera-dfl.c
> @@ -134,7 +134,7 @@ static int dfl_spi_altera_probe(struct dfl_device *dfl_dev)
>  	if (!master)
>  		return -ENOMEM;
>  
> -	master->bus_num = dfl_dev->id;
> +	master->bus_num = -1;
>  
>  	hw = spi_master_get_devdata(master);
>  
> diff --git a/drivers/spi/spi-altera-platform.c b/drivers/spi/spi-altera-platform.c
> index f7a7c14e3679..65147aae82a1 100644
> --- a/drivers/spi/spi-altera-platform.c
> +++ b/drivers/spi/spi-altera-platform.c
> @@ -48,7 +48,7 @@ static int altera_spi_probe(struct platform_device *pdev)
>  		return err;
>  
>  	/* setup the master state. */
> -	master->bus_num = pdev->id;
> +	master->bus_num = -1;
>  
>  	if (pdata) {
>  		if (pdata->num_chipselect > ALTERA_SPI_MAX_CS) {
Mark Brown Oct. 20, 2021, 1:02 a.m. UTC | #2
On Mon, Oct 18, 2021 at 05:27:38PM -0700, Russ Weight wrote:

> If there are no concerns with this patch, it will need to have the "cc stable"
> tag added for earlier kernels.

This feels like a risky change to introduce into stable kernels
given that while people shouldn't rely on stable number
assignment they may well have done so.  I'll skip the stable tag
though if you want to request a backport with the stable team
they might be OK with it - it definitely feels like it should
have a discussion rather than just going in with a tag.
Xu Yilun Oct. 20, 2021, 1:35 a.m. UTC | #3
On Mon, Oct 18, 2021 at 05:27:38PM -0700, Russ Weight wrote:
> If there are no concerns with this patch, it will need to have the "cc stable"
> tag added for earlier kernels.

The code is good to me, and please add the cc stable tag.

> 
> I don't think it needs a "Fixes" tag, as the problem has been there since the
> driver was introduced.

You could add the Fixes tag for the patches that introduce the drivers.

> 
> Thanks,
> - Russ
> 
> On 10/18/21 5:24 PM, Russ Weight wrote:
> > The spi-altera driver has two flavors: platform and dfl. I'm seeing
> > a case where I have both device types in the same machine, and they
> > are conflicting on the SPI ID:
> >
> > ... kernel: couldn't get idr
> > ... kernel: WARNING: CPU: 28 PID: 912 at drivers/spi/spi.c:2920 spi_register_controller.cold+0x84/0xc0a
> >
> > Both the platform and dfl drivers use the parent's driver ID as the SPI
> > ID. In the error case, the parent devices are dfl_dev.4 and
> > subdev_spi_altera.4.auto. When the second spi-master is created, the
> > failure occurs because the SPI ID of 4 has already been allocated.
> >
> > Change the ID allocation to dynamic (by initializing bus_num to -1) to
> > avoid duplicate SPI IDs.
> >
> > Signed-off-by: Russ Weight <russell.h.weight@intel.com>

Please add the tags and then,
Acked-by: Xu Yilun <yilun.xu@intel.com>

Thanks,
Yilun

> > ---
> >  drivers/spi/spi-altera-dfl.c      | 2 +-
> >  drivers/spi/spi-altera-platform.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/spi/spi-altera-dfl.c b/drivers/spi/spi-altera-dfl.c
> > index 44fc9ee13fc7..ca40923258af 100644
> > --- a/drivers/spi/spi-altera-dfl.c
> > +++ b/drivers/spi/spi-altera-dfl.c
> > @@ -134,7 +134,7 @@ static int dfl_spi_altera_probe(struct dfl_device *dfl_dev)
> >  	if (!master)
> >  		return -ENOMEM;
> >  
> > -	master->bus_num = dfl_dev->id;
> > +	master->bus_num = -1;
> >  
> >  	hw = spi_master_get_devdata(master);
> >  
> > diff --git a/drivers/spi/spi-altera-platform.c b/drivers/spi/spi-altera-platform.c
> > index f7a7c14e3679..65147aae82a1 100644
> > --- a/drivers/spi/spi-altera-platform.c
> > +++ b/drivers/spi/spi-altera-platform.c
> > @@ -48,7 +48,7 @@ static int altera_spi_probe(struct platform_device *pdev)
> >  		return err;
> >  
> >  	/* setup the master state. */
> > -	master->bus_num = pdev->id;
> > +	master->bus_num = -1;
> >  
> >  	if (pdata) {
> >  		if (pdata->num_chipselect > ALTERA_SPI_MAX_CS) {
Xu Yilun Oct. 20, 2021, 1:39 a.m. UTC | #4
On Wed, Oct 20, 2021 at 02:02:30AM +0100, Mark Brown wrote:
> On Mon, Oct 18, 2021 at 05:27:38PM -0700, Russ Weight wrote:
> 
> > If there are no concerns with this patch, it will need to have the "cc stable"
> > tag added for earlier kernels.
> 
> This feels like a risky change to introduce into stable kernels
> given that while people shouldn't rely on stable number
> assignment they may well have done so.  I'll skip the stable tag
> though if you want to request a backport with the stable team
> they might be OK with it - it definitely feels like it should
> have a discussion rather than just going in with a tag.

It makes sense. Just skip my stable tag comment.

Thanks,
Yilun
Mark Brown Oct. 20, 2021, 10:40 a.m. UTC | #5
On Mon, 18 Oct 2021 17:24:01 -0700, Russ Weight wrote:
> The spi-altera driver has two flavors: platform and dfl. I'm seeing
> a case where I have both device types in the same machine, and they
> are conflicting on the SPI ID:
> 
> ... kernel: couldn't get idr
> ... kernel: WARNING: CPU: 28 PID: 912 at drivers/spi/spi.c:2920 spi_register_controller.cold+0x84/0xc0a
> 
> [...]

Applied, thanks!

[1/1] spi: altera: Change to dynamic allocation of spi id
      commit: f09f6dfef8ce7b70a240cf83811e2b1909c3e47b

Best regards,
Mark Brown Oct. 20, 2021, 11:28 a.m. UTC | #6
On Mon, 18 Oct 2021 17:24:01 -0700, Russ Weight wrote:
> The spi-altera driver has two flavors: platform and dfl. I'm seeing
> a case where I have both device types in the same machine, and they
> are conflicting on the SPI ID:
> 
> ... kernel: couldn't get idr
> ... kernel: WARNING: CPU: 28 PID: 912 at drivers/spi/spi.c:2920 spi_register_controller.cold+0x84/0xc0a
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: altera: Change to dynamic allocation of spi id
      commit: f09f6dfef8ce7b70a240cf83811e2b1909c3e47b

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
diff mbox series

Patch

diff --git a/drivers/spi/spi-altera-dfl.c b/drivers/spi/spi-altera-dfl.c
index 44fc9ee13fc7..ca40923258af 100644
--- a/drivers/spi/spi-altera-dfl.c
+++ b/drivers/spi/spi-altera-dfl.c
@@ -134,7 +134,7 @@  static int dfl_spi_altera_probe(struct dfl_device *dfl_dev)
 	if (!master)
 		return -ENOMEM;
 
-	master->bus_num = dfl_dev->id;
+	master->bus_num = -1;
 
 	hw = spi_master_get_devdata(master);
 
diff --git a/drivers/spi/spi-altera-platform.c b/drivers/spi/spi-altera-platform.c
index f7a7c14e3679..65147aae82a1 100644
--- a/drivers/spi/spi-altera-platform.c
+++ b/drivers/spi/spi-altera-platform.c
@@ -48,7 +48,7 @@  static int altera_spi_probe(struct platform_device *pdev)
 		return err;
 
 	/* setup the master state. */
-	master->bus_num = pdev->id;
+	master->bus_num = -1;
 
 	if (pdata) {
 		if (pdata->num_chipselect > ALTERA_SPI_MAX_CS) {