From:   <Tharunkumar.Pasumarthi@microchip.com>
To:     <gregkh@linuxfoundation.org>
CC:     <arnd@arndb.de>, <linux-kernel@vger.kernel.org>,
        <linux-gpio@vger.kernel.org>, <UNGLinuxDriver@microchip.com>
Subject: RE: [PATCH v4 char-misc-next] misc: microchip: pci1xxxx: Add
 OTP/EEPROM driver for the pci1xxxx switch
Thread-Topic: [PATCH v4 char-misc-next] misc: microchip: pci1xxxx: Add
 OTP/EEPROM driver for the pci1xxxx switch
Thread-Index: AQHZPED0IEC8iEmq9kS9Rw5j6ZWLp67GT+IAgALkDmA=
Date:   Sat, 11 Feb 2023 05:07:43 +0000
Message-ID: <PH7PR11MB59588BF687D7D2EDAC36E4899BDF9@PH7PR11MB5958.namprd11.prod.outlook.com>
References: <20230209044237.3927293-1-tharunkumar.pasumarthi@microchip.com>
 <Y+S05tQ5e5pE9/v0@kroah.com>
In-Reply-To: <Y+S05tQ5e5pE9/v0@kroah.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?l/B7nrZGdWkJlH7nuLxkBJGGPCxZQm8wfLLPT21AmXdJrKiQA1MrGkrpwFjx?=
 =?us-ascii?Q?OihVQjSf6cOYsIAEhyF7NnrMPP8XOch9osyzeVr4UyI9QlInZkIlqX+yNk5t?=
 =?us-ascii?Q?+dErSlH9HGQvfuGXlyW4kvYzAkvKonzv0opr9tnNZ/YbVhtCIVPiNa3+1pI1?=
 =?us-ascii?Q?G2LY4f4m3Y7aO4PV7k9Z12yzwQBFvJpxf5u/+FxQTMEOI1NXZD8ui0+i0Edw?=
 =?us-ascii?Q?zAmM8qN/Wk11rEUhOu+yKd2QmIGr+7gfrGdPu4ZfIOKluiPH+h1sO5axYTEI?=
 =?us-ascii?Q?6RcwV1aVuVUVyMqoKXu2+udi2hn4ucgNrPi+nzB+JR/wX2sUIkKc756twr1Z?=
 =?us-ascii?Q?JW7q6qUHHdhNy4+IJQd37mOkN06H7H5rMPeG1YCRPf1IvpYQET862SUD2itc?=
 =?us-ascii?Q?Vjvwqy8aHI+CSEbOP6Unw8cJus1vvmO6dvTftqxVEjmGjeQpD678k+g1rWOB?=
 =?us-ascii?Q?hCavScgsaEQIwUCY2TUIIbwUTstEitjLx9ehGRDXz7sARpXEumVVkDfdPvHu?=
 =?us-ascii?Q?EWMTdWW7PFg+xc0yEaWL8fyNnkrrY3HiGPnoDkrlSJTPcKozkqgEA4RRYz7r?=
 =?us-ascii?Q?yvUOxe71LuJMmkmsQE/ZkZ9YE3H88IQpmAQEx6pJhD+0Hj+iQ6hnD51GbUwN?=
 =?us-ascii?Q?RRmE0feGRu90Sq8NL8mqms4UV5NILEucv9bYYe8RM5xN34+dfO9xAeQ5bfTo?=
 =?us-ascii?Q?PrbRn4CGibIJmHQDTu4ZKQFbFGOmrXU7EMc5wWBapCF4tHuL5MHCPrWuBhuz?=
 =?us-ascii?Q?k5ztytQiP72pAd1sC0Lywr1mdBFXaUF+5XgcArHLi1xVA5vixObL8+O2rOkG?=
 =?us-ascii?Q?Lz0gzC7oWf633fuwaCLoxs6gWl+LuuyuEawlr6Elycfsh6xOjtUs6cOn3EXJ?=
 =?us-ascii?Q?6rFM5VW9DFORpSjW08/OGXunndDRCk3VLVH5xAOVmIULQ8wwOFq0LGYMMm+c?=
 =?us-ascii?Q?+KoeRbTNNqhRohHJnjeOzCKt1eVvWroydaKIS8IdoHiMYlMXVKIRPtI76ARd?=
 =?us-ascii?Q?xQ3h8ShvH3YLjG8qgJbixO1wA8Vv00Zm8nQ3fVhGOhY3iCVWzW2m+pk+Ehj9?=
 =?us-ascii?Q?U2/VqsmKLLo/915ltIMXFDQ38J2iVQEjXmCJJxaJ/H429sF5qa6KxseqSniL?=
 =?us-ascii?Q?Ct37Cnrmswn96cvcZlZuhp6wBakCC9Q+3L1SvgCHtFNKF/IYost7cn6l9DBt?=
 =?us-ascii?Q?rQiEXgp15bW92OWug/o4Lc0uREHkx5qdjFgRDDX9Jb5dNFM+zz6qUDlr2NGi?=
 =?us-ascii?Q?qAN3p2ht849QViqNxOXTj413R9GkAXn2dOjFNSgX/UZXHNuzR6DqnPk+hUYf?=
 =?us-ascii?Q?9QERQli/WtIaNRBE7v4yIqzqGSd9Brb+reQ0N5MZD1QNDQYFFH88ghSMbcHs?=
 =?us-ascii?Q?Q5KXJj+cozu5HMlO51rVg6MJVU1/iqewaFK/mBy1crvoW9VF2wVkLPXqPvrv?=
 =?us-ascii?Q?Qbjoplev81P4L2ym6x91ZmS8KYjBM9bv54VbtAcMRCFw00oHymNbW37yqanJ?=
 =?us-ascii?Q?8XrfJVmizo0jF4PY4X+S9OXs0CgF0hyvU52EtQwTvTrPvISJnbFRZfIxgVdN?=
 =?us-ascii?Q?biYfuIq4r4eOFVQCJh6Nj7hrWaBBBGkoS5IGtQFCpDpzWYaRdUaFgFASMukZ?=
 =?us-ascii?Q?JK5x2eJKBDEvLAvcV1kuMZE=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH7PR11MB5958.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5c59ed1a-4685-491e-c046-08db0bede884
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Feb 2023 05:07:43.8757
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 9M4ZQ0759WXc0G3aXAyUqlJ49077pgNHRAHHwOEghFHwDPRsHUJbwyJ0iWssnFgd766HKOxl7foGlmYaiChqkGSgxL25w3UWKoKTKuSz9WTV4Iv9kk6Ff3XpQG1BABLb
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5438
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Thursday, February 9, 2023 2:25 PM
> To: Tharunkumar Pasumarthi - I67821
> <Tharunkumar.Pasumarthi@microchip.com>
> EXTERNAL EMAIL: Do not click links or open attachments unless you know th=
e
> content is safe
>=20
> > +     } while (data & EEPROM_CMD_EPC_BUSY_BIT);
>=20
> That's a very busy "sit and spin" loop here, what happens if the read of =
the
> bit never actually succeeds?  You just locked up the system with no way t=
o
> interrupt it :(
>=20
> Please provide some sort of timeout, or way to break out of this.
>=20
> > +
> > +     if (data & EEPROM_CMD_EPC_TIMEOUT_BIT) {
> > +             dev_err(&priv->pdev->dev, "EEPROM write timed out\n");
>=20
> How can the timeout bit happen if the busy bit was still set?
>=20
> And what can userspace do about this if it is reported?

Hi Greg,
If EEPROM_CMD_EPC_BUSY_BIT is set for more than 30ms, it will be cleared au=
tomatically by the hardware logic and EEPROM_CMD_EPC_TIMEOUT_BIT bit will b=
e set to indicate the timeout. User space application will inform user abou=
t timeout on EEPROM write/read when this error occurs.

>=20
> > +             return -EFAULT;
>=20
> This return value is ONLY for when we have memory faults from reading
> to/from userspace and the kernel.  It's not a valid return value for a de=
vice
> error, sorry.  -EIO maybe?
>=20
> You return this error in a number of other places in the driver that shou=
ldn't,
> please fix this up.

Okay.

Thanks,
Tharun Kumar P