From: =?iso-8859-2?Q?Horia_Geant=E3?= <horia.geanta@nxp.com>
To: Stuart Yoder <stuyoder@gmail.com>
CC: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        Roy Pledge <roy.pledge@nxp.com>,
        "devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Haiying Wang <haiying.wang@nxp.com>,
        "Ruxandra Ioana Radulescu" <ruxandra.radulescu@nxp.com>
Subject: Re: [PATCH] bus: fsl-mc: dpio: fix alter FQ state command
Thread-Topic: [PATCH] bus: fsl-mc: dpio: fix alter FQ state command
Thread-Index: AQHSuqONnJfZxc2Qd0qDSM1Jzajikg==
Date: Fri, 21 Apr 2017 13:50:24 +0000
Message-ID: <VI1PR0401MB2591DAF5E8813DD27C7AA4F8981A0@VI1PR0401MB2591.eurprd04.prod.outlook.com>
References: <20170421120018.22232-1-horia.geanta@nxp.com>
 <CAEac7tYLy0R14mPprYYxT5EfA=RDDFQoHQThbXMrAVb70uWCcg@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="iso-8859-2"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Apr 2017 13:50:24.4535
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0401MB2560
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v3LDodPL015216
Content-Length: 1430
Lines: 38

On 4/21/2017 4:31 PM, Stuart Yoder wrote:
> On Fri, Apr 21, 2017 at 7:00 AM, Horia Geant? <horia.geanta@nxp.com> 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? <horia.geanta@nxp.com>
>> ---
>>  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