When checking the response verb, the valid bit should be masked out,
since its value flips depending on what Response Register
(RR0 /RR1) it's been read from.
Fixes: 321eecb06bfb ("bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2")
Signed-off-by: Horia Geantă <[email protected]>
---
drivers/staging/fsl-mc/bus/dpio/qbman-portal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c b/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
index 2a3ea29d9b43..5d020fb98c66 100644
--- a/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
+++ b/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
@@ -963,7 +963,7 @@ int qbman_swp_alt_fq_state(struct qbman_swp *s, u32 fqid,
}
/* Decode the outcome */
- WARN_ON(r->verb != alt_fq_verb);
+ WARN_ON((r->verb & 0x7f) != alt_fq_verb);
/* Determine success or failure */
if (unlikely(r->rslt != QBMAN_MC_RSLT_OK)) {
--
2.12.0.264.gd6db3f216544
On Fri, Apr 21, 2017 at 7:00 AM, Horia Geantă <[email protected]> wrote:
>
> When checking the response verb, the valid bit should be masked out,
> since its value flips depending on what Response Register
> (RR0 /RR1) it's been read from.
>
> Fixes: 321eecb06bfb ("bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2")
> Signed-off-by: Horia Geantă <[email protected]>
> ---
> drivers/staging/fsl-mc/bus/dpio/qbman-portal.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c b/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
> index 2a3ea29d9b43..5d020fb98c66 100644
> --- a/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
> +++ b/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
> @@ -963,7 +963,7 @@ int qbman_swp_alt_fq_state(struct qbman_swp *s, u32 fqid,
> }
>
> /* Decode the outcome */
> - WARN_ON(r->verb != alt_fq_verb);
> + WARN_ON((r->verb & 0x7f) != alt_fq_verb);
Don't use magic constants like that. Create a #define for the valid bit.
Thanks,
Stuart
On 4/21/2017 4:31 PM, Stuart Yoder wrote:
> On Fri, Apr 21, 2017 at 7:00 AM, Horia Geant? <[email protected]> wrote:
>>
>> When checking the response verb, the valid bit should be masked out,
>> since its value flips depending on what Response Register
>> (RR0 /RR1) it's been read from.
>>
>> Fixes: 321eecb06bfb ("bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2")
>> Signed-off-by: Horia Geant? <[email protected]>
>> ---
>> drivers/staging/fsl-mc/bus/dpio/qbman-portal.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c b/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
>> index 2a3ea29d9b43..5d020fb98c66 100644
>> --- a/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
>> +++ b/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
>> @@ -963,7 +963,7 @@ int qbman_swp_alt_fq_state(struct qbman_swp *s, u32 fqid,
>> }
>>
>> /* Decode the outcome */
>> - WARN_ON(r->verb != alt_fq_verb);
>> + WARN_ON((r->verb & 0x7f) != alt_fq_verb);
>
> Don't use magic constants like that. Create a #define for the valid bit.
>
Just looking at the surrounding code, this seems the preferred way to
mask the valid bit.
Though there are two defines that could be used in this case:
#define QB_VALID_BIT ((u32)0x80)
#define QBMAN_RESULT_MASK 0x7f
Let me know whether keeping code similar would be preferred or not.
Thanks,
Horia
On Fri, Apr 21, 2017 at 01:50:24PM +0000, Horia Geantă wrote:
> Let me know whether keeping code similar would be preferred or not.
>
Keeping the code similar to the surrounding is never a valid argument
but especially not in staging/. If only 1 line is nice in the whole
file, at least that's better than nothing.
regards,
dan carpenter
When checking the response verb, the valid bit should be masked out,
since its value flips depending on what Response Register
(RR0 /RR1) it's been read from.
Fixes: 321eecb06bfb ("bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2")
Signed-off-by: Horia Geantă <[email protected]>
---
v2: use QBMAN_RESULT_MASK instead of hard-coded mask
drivers/staging/fsl-mc/bus/dpio/qbman-portal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c b/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
index 2a3ea29d9b43..7988612aaecf 100644
--- a/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
+++ b/drivers/staging/fsl-mc/bus/dpio/qbman-portal.c
@@ -963,7 +963,7 @@ int qbman_swp_alt_fq_state(struct qbman_swp *s, u32 fqid,
}
/* Decode the outcome */
- WARN_ON(r->verb != alt_fq_verb);
+ WARN_ON((r->verb & QBMAN_RESULT_MASK) != alt_fq_verb);
/* Determine success or failure */
if (unlikely(r->rslt != QBMAN_MC_RSLT_OK)) {
--
2.12.0.264.gd6db3f216544
On Sat, Apr 22, 2017 at 09:44:49AM +0300, Horia Geantă wrote:
> When checking the response verb, the valid bit should be masked out,
> since its value flips depending on what Response Register
> (RR0 /RR1) it's been read from.
>
> Fixes: 321eecb06bfb ("bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2")
> Signed-off-by: Horia Geantă <[email protected]>
> ---
> v2: use QBMAN_RESULT_MASK instead of hard-coded mask
Why are you not using 'staging' in your subject line?
thanks,
greg k-h
On 4/28/2017 1:32 PM, Greg KH wrote:
> On Sat, Apr 22, 2017 at 09:44:49AM +0300, Horia Geant? wrote:
>> When checking the response verb, the valid bit should be masked out,
>> since its value flips depending on what Response Register
>> (RR0 /RR1) it's been read from.
>>
>> Fixes: 321eecb06bfb ("bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2")
>> Signed-off-by: Horia Geant? <[email protected]>
>> ---
>> v2: use QBMAN_RESULT_MASK instead of hard-coded mask
>
> Why are you not using 'staging' in your subject line?
>
I was not aware this is required and followed the statistics:
git log --oneline drivers/staging/fsl-mc/bus/dpio/
141a10aead9c staging: fsl-mc/dpio: Fix early writing of valid bit
993fec7e11ca bus: fsl-mc: dpio: add the DPAA2 DPIO object driver
780b626323d7 bus: fsl-mc: dpio: add the DPAA2 DPIO service interface
321eecb06bfb bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2
2704aedb5c21 bus: fsl-mc: dpio: add APIs for DPIO objects
0e6437941e44 bus: fsl-mc: dpio: add DPIO driver overview document
Do I have to resubmit?
Thanks,
Horia
On Fri, Apr 28, 2017 at 10:47:14AM +0000, Horia Geantă wrote:
> On 4/28/2017 1:32 PM, Greg KH wrote:
> > On Sat, Apr 22, 2017 at 09:44:49AM +0300, Horia Geantă wrote:
> >> When checking the response verb, the valid bit should be masked out,
> >> since its value flips depending on what Response Register
> >> (RR0 /RR1) it's been read from.
> >>
> >> Fixes: 321eecb06bfb ("bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2")
> >> Signed-off-by: Horia Geantă <[email protected]>
> >> ---
> >> v2: use QBMAN_RESULT_MASK instead of hard-coded mask
> >
> > Why are you not using 'staging' in your subject line?
> >
> I was not aware this is required and followed the statistics:
>
> git log --oneline drivers/staging/fsl-mc/bus/dpio/
> 141a10aead9c staging: fsl-mc/dpio: Fix early writing of valid bit
> 993fec7e11ca bus: fsl-mc: dpio: add the DPAA2 DPIO object driver
> 780b626323d7 bus: fsl-mc: dpio: add the DPAA2 DPIO service interface
> 321eecb06bfb bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2
> 2704aedb5c21 bus: fsl-mc: dpio: add APIs for DPIO objects
> 0e6437941e44 bus: fsl-mc: dpio: add DPIO driver overview document
>
> Do I have to resubmit?
No, I have edited the subject, it should be "staging: fsl-mc: bus: ..."
for future patches.
thanks,
greg k-h