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@nxp.com" <peter.chen@nxp.com>
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+sw
Date:   Fri, 25 Jun 2021 04:23:08 +0000
Message-ID: <BYAPR07MB538179F3E10205B43C52DB19DD069@BYAPR07MB5381.namprd07.prod.outlook.com>
References: <d12bfcc9cbffb89e27b120668821b3c4f09b6755.1624390584.git.christophe.jaillet@wanadoo.fr>
In-Reply-To: <d12bfcc9cbffb89e27b120668821b3c4f09b6755.1624390584.git.christophe.jaillet@wanadoo.fr>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?jmPeXUe4yH/VUXGnoI+Y/W9I2ESdZQVynCDS0rmgpCKO55dYWwcrV87XJWck?=
 =?us-ascii?Q?nHR9Bj8LRGB29V5mQe9FDzujqHxLLzclIrVgsJlF0ZYMpjsxic5qMqLwgAeg?=
 =?us-ascii?Q?x4FMQ0k557J4DTm1rvj/mfbE8REH8LFstHV5edfZoBzj9rFDOfJl3Zgyegtr?=
 =?us-ascii?Q?B0/ulGG2JlOxttazr/AUnABy4Vy061hDCb1dR8SkPK4R7s+RQK6veo8tePCE?=
 =?us-ascii?Q?21eFOWbsFZnCFcrAjt1JCfIPBykrfmuwNU6FQHnLPOglcfuhYm9YsQ+ObwUX?=
 =?us-ascii?Q?smM3js7hGDEoNCwKHx/tgro9JwKk/6cCqqRb6tJG0jNxVXLaM7om+aj9ijXD?=
 =?us-ascii?Q?933okY2JLgOfATJSFoJu0Hr9xyPH9xHURexMaTTKKKM7au/bStQY2HUei5H4?=
 =?us-ascii?Q?PMIXin3nvjhJv3h8Z/IgtMbsp0HRhC6yBHptU0RxSXKWCAZ7NJOIMOm6woF/?=
 =?us-ascii?Q?tMIQRE0j5jB4eWmhCIqIpz4OrP4x5l8NPMnJUvuqsGjc2JCh/s6Mlq+ExWjC?=
 =?us-ascii?Q?r01t2V3G3CvCKMO3d27BtdMOxBtdFWNfvFgAv2ipc4YjVQvfmIM1Bc+XX97i?=
 =?us-ascii?Q?OWO9Pe0emX4M/shIgZkNuWIO5Amz8SZtQL3kR0bGLLvxLSrIjGxVAQInGoJO?=
 =?us-ascii?Q?BJyidAKpIktDnNLfH+pXZ2mCR277tksHmd4ss7xbBKnDHvdJOoL2ye+FlBBQ?=
 =?us-ascii?Q?lmSUdwXJ4vPVzxND5lCoN0g8MAI78LeTY0LJfdyUnT5KfHQF8Q7a92zYl8rF?=
 =?us-ascii?Q?Yw8xwqvueN0GhMDRcGIbA3BygTqd20y0AteXGdsgnSCnCQi/HoOyNqriTNTb?=
 =?us-ascii?Q?KSyCgfPeO2bXuuTr8gfrfWw6mZK9eyM3jQdwbZnlB+nZu9OQhrfmvPdlPlEk?=
 =?us-ascii?Q?Mv2ai9pG4i9xKkyPO+ouK+lGcXDvT/6GaWGjxwYA4AorSQA4H6sgsYKtJ0eP?=
 =?us-ascii?Q?fpr6+sEyQeMpv2G1AFxI+gS/Y/Biq6DJcmgvJUaBKEprObWKiDpdZjHPyVj7?=
 =?us-ascii?Q?4aUE2n7vLLLZLHREdg/WWdZ8VGWif4vbYddGq9CEV1T3RhIeUJoA/U7Qb+zN?=
 =?us-ascii?Q?43GAlqzsOnVHVSay7wsBHesZGf9yQB4WDfT2wtSTkaQ4wMFSvzPsiExtaVZi?=
 =?us-ascii?Q?72A283vFP8MLPsWIxisJMxR6f6dfPanHZ7+SnNipTtsh0eKY/vQag00T5Uft?=
 =?us-ascii?Q?PUMk5QF8H9f6I31eV9BCszSoDHs7yJM/DuGz2uMCqVgrabgjPU6lNMtIFrJv?=
 =?us-ascii?Q?oOxwrV7vaunYGRJssoEQaOyYYkX0ON1l/m/ysqSx8M47b503jQzimILGC82e?=
 =?us-ascii?Q?JyQeglaHeJXj5GF7fj7xKu9/?=
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: 183398e8-18dc-4108-d2cd-08d93790efe7
X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Jun 2021 04:23:08.7522
 (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: CQ6XlUyG9coNe3ycgNzBX2cJM+EF/SR/HjS5cyGV7JII+mmx2RvBAqeCYEQXz1zbo1UWzSngvHrGGUfPUmwg8psaz/uRpCZoSGaX9aGySJo=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR07MB8662
X-Proofpoint-ORIG-GUID: zWX339g9tCOL444kWzlaAbXX4lfcYbtd
X-Proofpoint-GUID: zWX339g9tCOL444kWzlaAbXX4lfcYbtd
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 malwarescore=0
 priorityscore=1501 impostorscore=0 spamscore=0 phishscore=0 suspectscore=0
 clxscore=1011 mlxlogscore=999 adultscore=0 mlxscore=0 bulkscore=0
 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2104190000 definitions=main-2106250023
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.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 driver=
")
>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.=20
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 fo=
und any
issue with this patch during testing so we can simplify these macros=20

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-ga=
dget.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