Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Pawel Laszczak <pawell@cadence.com>
To:     Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        Peter Chen <peter.chen@kernel.org>
CC:     "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kernel-janitors@vger.kernel.org" <kernel-janitors@vger.kernel.org>
Subject: RE: [PATCH] usb: cdnsp: Fix the IMAN_IE_SET and IMAN_IE_CLEAR macro.
Thread-Topic: [PATCH] usb: cdnsp: Fix the IMAN_IE_SET and IMAN_IE_CLEAR macro.
Thread-Index: AQHXZ54aqs4YpaHOu0aHRPbIczk5K6skH+swgAAIeDA=
Date:   Fri, 25 Jun 2021 04:39:13 +0000
Message-ID: <BYAPR07MB53812DBF7D5860FD4DEFD485DD069@BYAPR07MB5381.namprd07.prod.outlook.com>
References: <d12bfcc9cbffb89e27b120668821b3c4f09b6755.1624390584.git.christophe.jaillet@wanadoo.fr>
 <BYAPR07MB538179F3E10205B43C52DB19DD069@BYAPR07MB5381.namprd07.prod.outlook.com>
In-Reply-To: <BYAPR07MB538179F3E10205B43C52DB19DD069@BYAPR07MB5381.namprd07.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?39WjQeFWSJ+iWjd+ewxP2SwxurTDy61vGVL6YcUQ6hTbOR/kOOBdRY3ZSXLf?=
 =?us-ascii?Q?FvVh2imSph3WXRt13762JUgj0n1E3QBpKji6462igQA1Py/0mHJtMdU74UkF?=
 =?us-ascii?Q?drjq+TKf3M3QZ7axiOdWbnHVRLvpTIptoMhjD4IzKPkhCrsME5yC2bwlHWvP?=
 =?us-ascii?Q?+P/YXPZOBUhsP1ltluq/kBCiZ3I6/v0LXxGD0a2kyP7FACmb+fbSmytbM1Gb?=
 =?us-ascii?Q?rflMQkRDqdW5vsgrECTVjKSj9h89ihq+TAmoa5evlyo25bLYs36hH0glpJh4?=
 =?us-ascii?Q?S04o2G3Drc4+yibNQkWdauGhcvZV1V6wnuOY2zm1P+adVB+7K7/Qma1kgHkF?=
 =?us-ascii?Q?wwagDnnPCzf63T+t/i8VTffZ7jfPFHSL+0TGGIxmY8aM/OOBRO4768UVhrxC?=
 =?us-ascii?Q?uYq6fcB/GanxxMTUPto+cta7PHBAw38jU5MEOzMTnWfFEP18sXEqPaOB/aO0?=
 =?us-ascii?Q?GCXq2PRgkVgSfGxj0YiOPW4snOtJTTQCirftdtrDvwJiAm20JhO3jDF8KGSw?=
 =?us-ascii?Q?iI/82grb6pqkZDPHEUlsudNstb1Y+wLuRGIxlJ6RYxpy7GI4bVNc58mPmPhw?=
 =?us-ascii?Q?BVvfT6wfa8+Rdeop7PG8KCZgIqGRXUTy+vaSIxVvIWHHDm64MKzth1UE7aP3?=
 =?us-ascii?Q?Gb5u4NmkJQt22kaPgIa+PMcXMQYSUzHF8GdS+d/Jxb9qzQ01G9/uWiOQNGi+?=
 =?us-ascii?Q?HnP42+zSPPJ3d+HpEjFzdX2+SsATz2HO9jm7yye+rHWs1USzE4DyzwDWGC43?=
 =?us-ascii?Q?R4zw8+/NHv6cHi+EyPHYODZ56UrQXhBS3Q7GZ2HbkxNjEKALZ0JZ3iA8DmCo?=
 =?us-ascii?Q?AngumE1aBrFU1w37Kp8Uk3BF0kfUOqKD1r8FS4uYQ53byM6ciufpRUXu/liX?=
 =?us-ascii?Q?/0xrdf3FloCA4KVQEtzDPq0M/voSs+Ft4JgsfVWgbcpi95Rsp1Y8LfRrgYvS?=
 =?us-ascii?Q?+yrR/uQ3nOXDhXdVEYiW3o2gv6uB6n9IdabUGOJb08aNdSlobqRqKOyTAr++?=
 =?us-ascii?Q?8OmoYwDUHa/CZFEzjIOhg38Vy/BL5+I0Jk/ZeD6pBLDr219z3Rf6Nruqk7cM?=
 =?us-ascii?Q?EfdmSapdIjkBt8Ht3sFyLRsYUfHiRPOBWL7qtTLkoNC1BOGcccT0akfd+rJ+?=
 =?us-ascii?Q?Bo4U0BPmc9bak2Ybvsg9KQH4XBkeTVBQqfx26dJu0KvK0qxpgN66O3VzUw4T?=
 =?us-ascii?Q?YDPAYaxgpmd4cJlGcd9HG3icPIVB1C3XFFyw61tx6KZ7LVNKAzujwhTDKIWq?=
 =?us-ascii?Q?hBpYmjueX481uOKGyGzgNjnaJX+ZpP1dMvLbmDmL954khcIt+jsHY5uw/hdW?=
 =?us-ascii?Q?17/uU5+V1Uxh0PuL/KC22m7W?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: cadence.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BYAPR07MB5381.namprd07.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: aa7c0255-cac4-4c2f-50b0-08d937932ec2
X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Jun 2021 04:39:13.2113
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: d36035c5-6ce6-4662-a3dc-e762e61ae4c9
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: HRJtxwD2qterXgQZcqrTPDerjZMyj5WVuDI/FALHzYu4tiQcbjGzzUUsJA7RhazG4f+mzzZzxjXFgetn7N3BrwrzWZXUw2+OODWk4/Qkcm8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR07MB8726
X-Proofpoint-GUID: qNHIn7RnD2guMeMh4Dql_gTqlcG9fIPi
X-Proofpoint-ORIG-GUID: qNHIn7RnD2guMeMh4Dql_gTqlcG9fIPi
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391,18.0.790
 definitions=2021-06-25_01:2021-06-24,2021-06-25 signatures=0
X-Proofpoint-Spam-Details: rule=outbound_check_notspam policy=outbound_check score=0 bulkscore=0
 impostorscore=0 lowpriorityscore=0 mlxscore=0 adultscore=0 malwarescore=0
 spamscore=0 mlxlogscore=999 phishscore=0 priorityscore=1501 clxscore=1015
 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2104190000 definitions=main-2106250025
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Corrected Peter Chen address - should be peter.chen@kernel.org

>>IMAN_IE is BIT(1), so these macro are respectively equivalent to BIT(1)
>>and 0, whatever the value of 'p'.
>>
>>The purpose was to set and reset a single bit in 'p'.
>>Fix these macros to do that correctly.
>>
>>Fixes: e93e58d27402 ("usb: cdnsp: Device side header file for CDNSP drive=
r")
>>Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>
>Acked-by: Pawel Laszczak <pawell@cadence.com>
>
>Currently the bit 1(RW) and bit 0 (W1toClr) are implemented.
>All other bits are not used and are reserved for future RW implementations=
.
>I was a bit afraid about bit 0 which is "write 1 to clear" but I haven't f=
ound any
>issue with this patch during testing so we can simplify these macros
>
>Thanks Christophe
>
>>---
>>This patch is speculative. It is not tested.
>>Please, review with care.
>>
>>Actually, the usage of these macros is:
>>   readl(somewhere)
>>   set or reset the IMAN_IE bit
>>   write(somewhere)
>>So it is likely that we want to preserve the other bits read. Otherwise,
>>the code could be much simpler.
>>---
>> drivers/usb/cdns3/cdnsp-gadget.h | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>>diff --git a/drivers/usb/cdns3/cdnsp-gadget.h b/drivers/usb/cdns3/cdnsp-g=
adget.h
>>index 783ca8ffde00..f740fa6089d8 100644
>>--- a/drivers/usb/cdns3/cdnsp-gadget.h
>>+++ b/drivers/usb/cdns3/cdnsp-gadget.h
>>@@ -383,8 +383,8 @@ struct cdnsp_intr_reg {
>> #define IMAN_IE			BIT(1)
>> #define IMAN_IP			BIT(0)
>> /* bits 2:31 need to be preserved */
>>-#define IMAN_IE_SET(p)		(((p) & IMAN_IE) | 0x2)
>>-#define IMAN_IE_CLEAR(p)	(((p) & IMAN_IE) & ~(0x2))
>>+#define IMAN_IE_SET(p)		((p) | IMAN_IE)
>>+#define IMAN_IE_CLEAR(p)	((p) & ~IMAN_IE)
>>
>> /* IMOD - Interrupter Moderation Register - irq_control bitmasks. */
>> /*
>>--
>>2.30.2
>
>--
>
>Regards,
>Pawel Laszczak