| Message ID | 20210625074213.654274-4-martin@geanix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fpga/mfd/hwmon: Initial support for Silicom N5010 PAC | expand |
It is good to me. On Fri, Jun 25, 2021 at 09:42:11AM +0200, Martin Hundebøll wrote: > From: Martin Hundebøll <mhu@silicom.dk> > > The Max10 BMC on the Silicom n5010 PAC is slightly different than the > existing BMC's, so use a dedicated feature revision detect it. > > Signed-off-by: Martin Hundebøll <mhu@silicom.dk> > --- > > Changes since v1: > * use feature revision from struct dfl_device instead of reading it > from io-mem > > drivers/spi/spi-altera-dfl.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/spi-altera-dfl.c b/drivers/spi/spi-altera-dfl.c > index 3e32e4fe5895..f6cf7c8d9dac 100644 > --- a/drivers/spi/spi-altera-dfl.c > +++ b/drivers/spi/spi-altera-dfl.c > @@ -111,6 +111,13 @@ static struct spi_board_info m10_bmc_info = { > .chip_select = 0, > }; > > +static struct spi_board_info m10_n5010_bmc_info = { > + .modalias = "m10-n5010", > + .max_speed_hz = 12500000, > + .bus_num = 0, > + .chip_select = 0, > +}; > + > static void config_spi_master(void __iomem *base, struct spi_master *master) > { > u64 v; > @@ -130,6 +137,7 @@ static void config_spi_master(void __iomem *base, struct spi_master *master) > > static int dfl_spi_altera_probe(struct dfl_device *dfl_dev) > { > + struct spi_board_info *board_info = &m10_bmc_info; > struct device *dev = &dfl_dev->dev; > struct spi_master *master; > struct altera_spi *hw; > @@ -172,9 +180,12 @@ static int dfl_spi_altera_probe(struct dfl_device *dfl_dev) > goto exit; > } > > - if (!spi_new_device(master, &m10_bmc_info)) { > + if (dfl_dev->revision == FME_FEATURE_REV_MAX10_SPI_N5010) > + board_info = &m10_n5010_bmc_info; > + > + if (!spi_new_device(master, board_info)) { > dev_err(dev, "%s failed to create SPI device: %s\n", > - __func__, m10_bmc_info.modalias); > + __func__, board_info->modalias); > } > > return 0; > -- > 2.31.0
On Fri, Jun 25, 2021 at 09:42:11AM +0200, Martin Hundebøll wrote: > From: Martin Hundebøll <mhu@silicom.dk> > > The Max10 BMC on the Silicom n5010 PAC is slightly different than the > existing BMC's, so use a dedicated feature revision detect it. > > Signed-off-by: Martin Hundebøll <mhu@silicom.dk> > --- > > Changes since v1: > * use feature revision from struct dfl_device instead of reading it > from io-mem > > drivers/spi/spi-altera-dfl.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/spi-altera-dfl.c b/drivers/spi/spi-altera-dfl.c > index 3e32e4fe5895..f6cf7c8d9dac 100644 > --- a/drivers/spi/spi-altera-dfl.c > +++ b/drivers/spi/spi-altera-dfl.c > @@ -111,6 +111,13 @@ static struct spi_board_info m10_bmc_info = { > .chip_select = 0, > }; > > +static struct spi_board_info m10_n5010_bmc_info = { > + .modalias = "m10-n5010", > + .max_speed_hz = 12500000, > + .bus_num = 0, > + .chip_select = 0, > +}; Is there no way to query the mc for version info? > + > static void config_spi_master(void __iomem *base, struct spi_master *master) > { > u64 v; > @@ -130,6 +137,7 @@ static void config_spi_master(void __iomem *base, struct spi_master *master) > > static int dfl_spi_altera_probe(struct dfl_device *dfl_dev) > { > + struct spi_board_info *board_info = &m10_bmc_info; > struct device *dev = &dfl_dev->dev; > struct spi_master *master; > struct altera_spi *hw; > @@ -172,9 +180,12 @@ static int dfl_spi_altera_probe(struct dfl_device *dfl_dev) > goto exit; > } > > - if (!spi_new_device(master, &m10_bmc_info)) { > + if (dfl_dev->revision == FME_FEATURE_REV_MAX10_SPI_N5010) > + board_info = &m10_n5010_bmc_info; Since this depends on the previous patch: Mark do you want to take both patches once they're reviewed? From what I can tell the BMC and HWMON don't directly depend on it, so taking them through SPI tree might be easiest. Alternatively I can provide a tag for the DFL change for you to pull. > + > + if (!spi_new_device(master, board_info)) { > dev_err(dev, "%s failed to create SPI device: %s\n", > - __func__, m10_bmc_info.modalias); > + __func__, board_info->modalias); > } > > return 0; > -- > 2.31.0 > - Moritz
On Mon, Jun 28, 2021 at 10:39:23AM -0700, Moritz Fischer wrote: > Since this depends on the previous patch: Mark do you want to take both > patches once they're reviewed? From what I can tell the BMC and HWMON > don't directly depend on it, so taking them through SPI tree might be > easiest. > Alternatively I can provide a tag for the DFL change for you to pull. Sure, I can do whichever - I guess me applying both is probably simplest?
On 28/06/2021 19.39, Moritz Fischer wrote: > On Fri, Jun 25, 2021 at 09:42:11AM +0200, Martin Hundebøll wrote: >> From: Martin Hundebøll<mhu@silicom.dk> >> >> The Max10 BMC on the Silicom n5010 PAC is slightly different than the >> existing BMC's, so use a dedicated feature revision detect it. >> >> Signed-off-by: Martin Hundebøll<mhu@silicom.dk> >> --- >> >> Changes since v1: >> * use feature revision from struct dfl_device instead of reading it >> from io-mem >> >> drivers/spi/spi-altera-dfl.c | 15 +++++++++++++-- >> 1 file changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/spi/spi-altera-dfl.c b/drivers/spi/spi-altera-dfl.c >> index 3e32e4fe5895..f6cf7c8d9dac 100644 >> --- a/drivers/spi/spi-altera-dfl.c >> +++ b/drivers/spi/spi-altera-dfl.c >> @@ -111,6 +111,13 @@ static struct spi_board_info m10_bmc_info = { >> .chip_select = 0, >> }; >> >> +static struct spi_board_info m10_n5010_bmc_info = { >> + .modalias = "m10-n5010", >> + .max_speed_hz = 12500000, >> + .bus_num = 0, >> + .chip_select = 0, >> +}; > Is there no way to query the mc for version info? Do you mean reading the BMC variant (i.e. n5010 / d5005 / n3000) from a register? Not in a uniform way across the different boards that I'm aware of. But isn't this what the DFL feature revision is meant for? // Martin
> On 28/06/2021 19.39, Moritz Fischer wrote: > > On Fri, Jun 25, 2021 at 09:42:11AM +0200, Martin Hundebøll wrote: > >> From: Martin Hundebøll<mhu@silicom.dk> > >> > >> The Max10 BMC on the Silicom n5010 PAC is slightly different than the > >> existing BMC's, so use a dedicated feature revision detect it. > >> > >> Signed-off-by: Martin Hundebøll<mhu@silicom.dk> > >> --- > >> > >> Changes since v1: > >> * use feature revision from struct dfl_device instead of reading it > >> from io-mem > >> > >> drivers/spi/spi-altera-dfl.c | 15 +++++++++++++-- > >> 1 file changed, 13 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/spi/spi-altera-dfl.c b/drivers/spi/spi-altera-dfl.c > >> index 3e32e4fe5895..f6cf7c8d9dac 100644 > >> --- a/drivers/spi/spi-altera-dfl.c > >> +++ b/drivers/spi/spi-altera-dfl.c > >> @@ -111,6 +111,13 @@ static struct spi_board_info m10_bmc_info = { > >> .chip_select = 0, > >> }; > >> > >> +static struct spi_board_info m10_n5010_bmc_info = { > >> + .modalias = "m10-n5010", > >> + .max_speed_hz = 12500000, > >> + .bus_num = 0, > >> + .chip_select = 0, > >> +}; > > Is there no way to query the mc for version info? > > Do you mean reading the BMC variant (i.e. n5010 / d5005 / n3000) from a > register? > > Not in a uniform way across the different boards that I'm aware of. But > isn't this what the DFL feature revision is meant for? If this is used to distinguish different boards, then revision (4bits?) may not be enough. New version DFH may be able to resolve this limitation, but it is always encouraged to have its own method to tell if possible, not depending on DFH, it makes this IP easy to be reused in non DFL case. Thanks Hao > > // Martin
On Tue, 29 Jun 2021, Wu, Hao wrote: >> On 28/06/2021 19.39, Moritz Fischer wrote: >>> On Fri, Jun 25, 2021 at 09:42:11AM +0200, Martin Hundebøll wrote: >>>> From: Martin Hundebøll<mhu@silicom.dk> >>>> >>>> The Max10 BMC on the Silicom n5010 PAC is slightly different than the >>>> existing BMC's, so use a dedicated feature revision detect it. >>>> >>>> Signed-off-by: Martin Hundebøll<mhu@silicom.dk> >>>> --- >>>> >>>> Changes since v1: >>>> * use feature revision from struct dfl_device instead of reading it >>>> from io-mem >>>> >>>> drivers/spi/spi-altera-dfl.c | 15 +++++++++++++-- >>>> 1 file changed, 13 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/spi/spi-altera-dfl.c b/drivers/spi/spi-altera-dfl.c >>>> index 3e32e4fe5895..f6cf7c8d9dac 100644 >>>> --- a/drivers/spi/spi-altera-dfl.c >>>> +++ b/drivers/spi/spi-altera-dfl.c >>>> @@ -111,6 +111,13 @@ static struct spi_board_info m10_bmc_info = { >>>> .chip_select = 0, >>>> }; >>>> >>>> +static struct spi_board_info m10_n5010_bmc_info = { >>>> + .modalias = "m10-n5010", >>>> + .max_speed_hz = 12500000, >>>> + .bus_num = 0, >>>> + .chip_select = 0, >>>> +}; >>> Is there no way to query the mc for version info? >> >> Do you mean reading the BMC variant (i.e. n5010 / d5005 / n3000) from a >> register? >> >> Not in a uniform way across the different boards that I'm aware of. But >> isn't this what the DFL feature revision is meant for? > > If this is used to distinguish different boards, then revision (4bits?) may not On the one hand, the revision is being used to distinguish the board. More precisely, the feature ID id determining the actual hardware involved, altera-spi connected to a particular indirect register mailbox. This is a different feature id used by the n3000 which has a different indirect register mailbox with a NIOS hanshake. So in this case the revision is being used to specify remote end of the SPI connection, d5005 BMC vs. n5010 BMC. I think in this case 4 bits is enough. We've only had two instances of this hardware in 5 years. Certainly any future instances of this hardware should have a register describing the remote end of the SPI connection. This hardware change would then require a new feature id. > be enough. New version DFH may be able to resolve this limitation, but it > is always encouraged to have its own method to tell if possible, not depending > on DFH, it makes this IP easy to be reused in non DFL case. > > Thanks > Hao > >> >> // Martin >
diff --git a/drivers/spi/spi-altera-dfl.c b/drivers/spi/spi-altera-dfl.c index 3e32e4fe5895..f6cf7c8d9dac 100644 --- a/drivers/spi/spi-altera-dfl.c +++ b/drivers/spi/spi-altera-dfl.c @@ -111,6 +111,13 @@ static struct spi_board_info m10_bmc_info = { .chip_select = 0, }; +static struct spi_board_info m10_n5010_bmc_info = { + .modalias = "m10-n5010", + .max_speed_hz = 12500000, + .bus_num = 0, + .chip_select = 0, +}; + static void config_spi_master(void __iomem *base, struct spi_master *master) { u64 v; @@ -130,6 +137,7 @@ static void config_spi_master(void __iomem *base, struct spi_master *master) static int dfl_spi_altera_probe(struct dfl_device *dfl_dev) { + struct spi_board_info *board_info = &m10_bmc_info; struct device *dev = &dfl_dev->dev; struct spi_master *master; struct altera_spi *hw; @@ -172,9 +180,12 @@ static int dfl_spi_altera_probe(struct dfl_device *dfl_dev) goto exit; } - if (!spi_new_device(master, &m10_bmc_info)) { + if (dfl_dev->revision == FME_FEATURE_REV_MAX10_SPI_N5010) + board_info = &m10_n5010_bmc_info; + + if (!spi_new_device(master, board_info)) { dev_err(dev, "%s failed to create SPI device: %s\n", - __func__, m10_bmc_info.modalias); + __func__, board_info->modalias); } return 0;