Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1617294 > unrolled thread
| Started by | matthew.gerlach@linux.intel.com |
|---|---|
| First post | 2017-04-05 21:10 +0200 |
| Last post | 2017-04-06 02:20 +0200 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH] fpga fr br: fix warning for unexpected version number matthew.gerlach@linux.intel.com - 2017-04-05 21:10 +0200
Re: [PATCH] fpga fr br: fix warning for unexpected version number Moritz Fischer <mdf@kernel.org> - 2017-04-05 22:20 +0200
Re: [PATCH] fpga fr br: fix warning for unexpected version number matthew.gerlach@linux.intel.com - 2017-04-06 00:30 +0200
Re: [PATCH] fpga fr br: fix warning for unexpected version number Moritz Fischer <mdf@kernel.org> - 2017-04-06 02:20 +0200
| From | matthew.gerlach@linux.intel.com |
|---|---|
| Date | 2017-04-05 21:10 +0200 |
| Subject | [PATCH] fpga fr br: fix warning for unexpected version number |
| Message-ID | <tsYtA-6BL-19@gated-at.bofh.it> |
From: Matthew Gerlach <matthew.gerlach@linux.intel.com> The value in the version register of the altera freeze bridge controller changed from the beta value of 2 to the value of 0xad000003 in the official release of the IP. This patch supports the old and new version numbers without printing an warning. Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com> --- drivers/fpga/altera-freeze-bridge.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/fpga/altera-freeze-bridge.c b/drivers/fpga/altera-freeze-bridge.c index 8dcd9fb..bdfd5eb 100644 --- a/drivers/fpga/altera-freeze-bridge.c +++ b/drivers/fpga/altera-freeze-bridge.c @@ -28,6 +28,7 @@ #define FREEZE_CSR_REG_VERSION 12 #define FREEZE_CSR_SUPPORTED_VERSION 2 +#define FREEZE_CSR_OFFICIAL_VERSION 0xad000003 #define FREEZE_CSR_STATUS_FREEZE_REQ_DONE BIT(0) #define FREEZE_CSR_STATUS_UNFREEZE_REQ_DONE BIT(1) @@ -241,10 +242,12 @@ static int altera_freeze_br_probe(struct platform_device *pdev) priv->enable = 1; revision = readl(priv->base_addr + FREEZE_CSR_REG_VERSION); - if (revision != FREEZE_CSR_SUPPORTED_VERSION) + if ((revision != FREEZE_CSR_SUPPORTED_VERSION) && + (revision != FREEZE_CSR_OFFICIAL_VERSION)) dev_warn(dev, - "%s Freeze Controller unexpected revision %d != %d\n", - __func__, revision, FREEZE_CSR_SUPPORTED_VERSION); + "%s unexpected revision 0x%x != 0x%x != 0x%x\n", + __func__, revision, FREEZE_CSR_SUPPORTED_VERSION, + FREEZE_CSR_OFFICIAL_VERSION); return fpga_bridge_register(dev, FREEZE_BRIDGE_NAME, &altera_freeze_br_br_ops, priv); -- 2.7.4
[toc] | [next] | [standalone]
| From | Moritz Fischer <mdf@kernel.org> |
|---|---|
| Date | 2017-04-05 22:20 +0200 |
| Message-ID | <tsZzj-7hF-1@gated-at.bofh.it> |
| In reply to | #1617294 |
Hi Matthew, On Wed, Apr 5, 2017 at 12:05 PM, <matthew.gerlach@linux.intel.com> wrote: > From: Matthew Gerlach <matthew.gerlach@linux.intel.com> > > The value in the version register of the altera freeze bridge > controller changed from the beta value of 2 to the > value of 0xad000003 in the official release of the IP. > This patch supports the old and new version numbers > without printing an warning. > > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com> > --- > drivers/fpga/altera-freeze-bridge.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/fpga/altera-freeze-bridge.c b/drivers/fpga/altera-freeze-bridge.c > index 8dcd9fb..bdfd5eb 100644 > --- a/drivers/fpga/altera-freeze-bridge.c > +++ b/drivers/fpga/altera-freeze-bridge.c > @@ -28,6 +28,7 @@ > #define FREEZE_CSR_REG_VERSION 12 > > #define FREEZE_CSR_SUPPORTED_VERSION 2 > +#define FREEZE_CSR_OFFICIAL_VERSION 0xad000003 > > #define FREEZE_CSR_STATUS_FREEZE_REQ_DONE BIT(0) > #define FREEZE_CSR_STATUS_UNFREEZE_REQ_DONE BIT(1) > @@ -241,10 +242,12 @@ static int altera_freeze_br_probe(struct platform_device *pdev) > priv->enable = 1; > > revision = readl(priv->base_addr + FREEZE_CSR_REG_VERSION); > - if (revision != FREEZE_CSR_SUPPORTED_VERSION) > + if ((revision != FREEZE_CSR_SUPPORTED_VERSION) && > + (revision != FREEZE_CSR_OFFICIAL_VERSION)) > dev_warn(dev, > - "%s Freeze Controller unexpected revision %d != %d\n", > - __func__, revision, FREEZE_CSR_SUPPORTED_VERSION); > + "%s unexpected revision 0x%x != 0x%x != 0x%x\n", > + __func__, revision, FREEZE_CSR_SUPPORTED_VERSION, > + FREEZE_CSR_OFFICIAL_VERSION); Maybe you actually wanna bail out if you read a random other value instead of what you expect instead of printing a warning. > > return fpga_bridge_register(dev, FREEZE_BRIDGE_NAME, > &altera_freeze_br_br_ops, priv); > -- > 2.7.4 > Cheers, Moritz
[toc] | [prev] | [next] | [standalone]
| From | matthew.gerlach@linux.intel.com |
|---|---|
| Date | 2017-04-06 00:30 +0200 |
| Message-ID | <tt1B7-8sp-5@gated-at.bofh.it> |
| In reply to | #1617324 |
On Wed, 5 Apr 2017, Moritz Fischer wrote: > Hi Matthew, > Hi Moritz, > On Wed, Apr 5, 2017 at 12:05 PM, <matthew.gerlach@linux.intel.com> wrote: >> From: Matthew Gerlach <matthew.gerlach@linux.intel.com> >> >> The value in the version register of the altera freeze bridge >> controller changed from the beta value of 2 to the >> value of 0xad000003 in the official release of the IP. >> This patch supports the old and new version numbers >> without printing an warning. >> >> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com> >> --- >> drivers/fpga/altera-freeze-bridge.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/fpga/altera-freeze-bridge.c b/drivers/fpga/altera-freeze-bridge.c >> index 8dcd9fb..bdfd5eb 100644 >> --- a/drivers/fpga/altera-freeze-bridge.c >> +++ b/drivers/fpga/altera-freeze-bridge.c >> @@ -28,6 +28,7 @@ >> #define FREEZE_CSR_REG_VERSION 12 >> >> #define FREEZE_CSR_SUPPORTED_VERSION 2 >> +#define FREEZE_CSR_OFFICIAL_VERSION 0xad000003 >> >> #define FREEZE_CSR_STATUS_FREEZE_REQ_DONE BIT(0) >> #define FREEZE_CSR_STATUS_UNFREEZE_REQ_DONE BIT(1) >> @@ -241,10 +242,12 @@ static int altera_freeze_br_probe(struct platform_device *pdev) >> priv->enable = 1; >> >> revision = readl(priv->base_addr + FREEZE_CSR_REG_VERSION); >> - if (revision != FREEZE_CSR_SUPPORTED_VERSION) >> + if ((revision != FREEZE_CSR_SUPPORTED_VERSION) && >> + (revision != FREEZE_CSR_OFFICIAL_VERSION)) >> dev_warn(dev, >> - "%s Freeze Controller unexpected revision %d != %d\n", >> - __func__, revision, FREEZE_CSR_SUPPORTED_VERSION); >> + "%s unexpected revision 0x%x != 0x%x != 0x%x\n", >> + __func__, revision, FREEZE_CSR_SUPPORTED_VERSION, >> + FREEZE_CSR_OFFICIAL_VERSION); > > Maybe you actually wanna bail out if you read a random other value > instead of what you > expect instead of printing a warning. I thought about making it an error if the version didn't match, but it was a dev_warn() before and that allowed folks to use the "old" driver with the new IP version. >> >> return fpga_bridge_register(dev, FREEZE_BRIDGE_NAME, >> &altera_freeze_br_br_ops, priv); >> -- >> 2.7.4 >> > > Cheers, > > Moritz >
[toc] | [prev] | [next] | [standalone]
| From | Moritz Fischer <mdf@kernel.org> |
|---|---|
| Date | 2017-04-06 02:20 +0200 |
| Message-ID | <tt3jz-18Z-1@gated-at.bofh.it> |
| In reply to | #1617402 |
Hi Matthew, On Wed, Apr 5, 2017 at 3:25 PM, <matthew.gerlach@linux.intel.com> wrote: >> Maybe you actually wanna bail out if you read a random other value >> instead of what you >> expect instead of printing a warning. > > > I thought about making it an error if the version didn't match, but it was a > dev_warn() before and that allowed folks to use the "old" driver with the > new IP version. Or some random other piece of logic in the FPGA that happens to be mapped to that address ;-) I agree we should've probably caught this in the initial review, but maybe we should change it. Cheers, Moritz
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web