* [PATCH linux dev-4.7] drivers: fsi: scom: Zero out user buffer first @ 2017-02-21 17:02 Eddie James 2017-02-22 3:21 ` Andrew Geissler 2017-02-22 3:28 ` Christopher Bostic 0 siblings, 2 replies; 4+ messages in thread From: Eddie James @ 2017-02-21 17:02 UTC (permalink / raw) To: openbmc; +Cc: joel, cbostic, Edward A. James From: "Edward A. James" <eajames@us.ibm.com> Scom data is just bit-wise OR'd with the user buffer, so it needs to be set to 0 first. Signed-off-by: Edward A. James <eajames@us.ibm.com> --- drivers/fsi/fsi-scom.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c index 51b3dc3..ed3b98e 100644 --- a/drivers/fsi/fsi-scom.c +++ b/drivers/fsi/fsi-scom.c @@ -85,6 +85,7 @@ static int get_scom(struct scom_device *scom_dev, uint64_t *value, uint32_t result, data; int rc; + *value = 0ULL; data = addr; rc = fsi_device_write(scom_dev->fsi_dev, SCOM_CMD_REG, &data, sizeof(uint32_t)); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH linux dev-4.7] drivers: fsi: scom: Zero out user buffer first 2017-02-21 17:02 [PATCH linux dev-4.7] drivers: fsi: scom: Zero out user buffer first Eddie James @ 2017-02-22 3:21 ` Andrew Geissler 2017-02-22 13:41 ` Joel Stanley 2017-02-22 3:28 ` Christopher Bostic 1 sibling, 1 reply; 4+ messages in thread From: Andrew Geissler @ 2017-02-22 3:21 UTC (permalink / raw) To: Eddie James; +Cc: OpenBMC Maillist, Edward A. James, cbostic On Tue, Feb 21, 2017 at 11:02 AM, Eddie James <eajames@linux.vnet.ibm.com> wrote: > From: "Edward A. James" <eajames@us.ibm.com> > > Scom data is just bit-wise OR'd with the user buffer, so it needs to be > set to 0 first. > > Signed-off-by: Edward A. James <eajames@us.ibm.com> > --- > drivers/fsi/fsi-scom.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c > index 51b3dc3..ed3b98e 100644 > --- a/drivers/fsi/fsi-scom.c > +++ b/drivers/fsi/fsi-scom.c > @@ -85,6 +85,7 @@ static int get_scom(struct scom_device *scom_dev, uint64_t *value, > uint32_t result, data; > int rc; > > + *value = 0ULL; > data = addr; > rc = fsi_device_write(scom_dev->fsi_dev, SCOM_CMD_REG, &data, > sizeof(uint32_t)); > -- > 1.8.3.1 > Looks good! Acked-by: Andrew Geissler <geissonator@gmail.com> Tested-by: Andrew Geissler <geissonator@gmail.com> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH linux dev-4.7] drivers: fsi: scom: Zero out user buffer first 2017-02-22 3:21 ` Andrew Geissler @ 2017-02-22 13:41 ` Joel Stanley 0 siblings, 0 replies; 4+ messages in thread From: Joel Stanley @ 2017-02-22 13:41 UTC (permalink / raw) To: Andrew Geissler Cc: Eddie James, Edward A. James, OpenBMC Maillist, Christopher Bostic On Wed, Feb 22, 2017 at 1:51 PM, Andrew Geissler <geissonator@gmail.com> wrote: > On Tue, Feb 21, 2017 at 11:02 AM, Eddie James > <eajames@linux.vnet.ibm.com> wrote: >> From: "Edward A. James" <eajames@us.ibm.com> >> >> Scom data is just bit-wise OR'd with the user buffer, so it needs to be >> set to 0 first. >> >> Signed-off-by: Edward A. James <eajames@us.ibm.com> >> --- >> drivers/fsi/fsi-scom.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c >> index 51b3dc3..ed3b98e 100644 >> --- a/drivers/fsi/fsi-scom.c >> +++ b/drivers/fsi/fsi-scom.c >> @@ -85,6 +85,7 @@ static int get_scom(struct scom_device *scom_dev, uint64_t *value, >> uint32_t result, data; >> int rc; >> >> + *value = 0ULL; >> data = addr; >> rc = fsi_device_write(scom_dev->fsi_dev, SCOM_CMD_REG, &data, >> sizeof(uint32_t)); We could do something more sensible, like apply the low 32 bits to value and then OR in the top half: rc = fsi_device_read(scom_dev->fsi_dev, SCOM_DATA1_REG, &result, sizeof(uint32_t)); if (rc) return rc; *value = result; rc = fsi_device_read(scom_dev->fsi_dev, SCOM_DATA0_REG, &result, sizeof(uint32_t)); if (rc) return rc; *value |= (uint64_t) result << 32; Or have result1 and result2, and then only save to value the once: *value = (result0 << 32) | result1; And then there are questions of endianess that must be considered before upstreaming. I have applied the fix as-is to dev-4.7. Please consider a cleaner fix when applying this patch to the series for upstream. Thanks for the review everyone. Most reviewed one liner OpenBMC has ever seen :D In all seriousness, keep it up, it's good to share the load. Cheers, Joel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH linux dev-4.7] drivers: fsi: scom: Zero out user buffer first 2017-02-21 17:02 [PATCH linux dev-4.7] drivers: fsi: scom: Zero out user buffer first Eddie James 2017-02-22 3:21 ` Andrew Geissler @ 2017-02-22 3:28 ` Christopher Bostic 1 sibling, 0 replies; 4+ messages in thread From: Christopher Bostic @ 2017-02-22 3:28 UTC (permalink / raw) To: Eddie James, openbmc; +Cc: Edward A. James On 2/21/17 11:02 AM, Eddie James wrote: > From: "Edward A. James" <eajames@us.ibm.com> > > Scom data is just bit-wise OR'd with the user buffer, so it needs to be > set to 0 first. > > Signed-off-by: Edward A. James <eajames@us.ibm.com> > --- > drivers/fsi/fsi-scom.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c > index 51b3dc3..ed3b98e 100644 > --- a/drivers/fsi/fsi-scom.c > +++ b/drivers/fsi/fsi-scom.c > @@ -85,6 +85,7 @@ static int get_scom(struct scom_device *scom_dev, uint64_t *value, > uint32_t result, data; > int rc; > > + *value = 0ULL; > data = addr; > rc = fsi_device_write(scom_dev->fsi_dev, SCOM_CMD_REG, &data, > sizeof(uint32_t)); Reviewed-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-02-22 13:41 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-21 17:02 [PATCH linux dev-4.7] drivers: fsi: scom: Zero out user buffer first Eddie James 2017-02-22 3:21 ` Andrew Geissler 2017-02-22 13:41 ` Joel Stanley 2017-02-22 3:28 ` Christopher Bostic
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.