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 |
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) {
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.
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) {
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
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,
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 --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) {
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(-)