Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Veerasenareddy Burru <vburru@marvell.com>
To:     Leon Romanovsky <leon@kernel.org>
CC:     "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Liron Himi <lironh@marvell.com>,
        Abhijit Ayarekar <aayarekar@marvell.com>,
        Sathesh B Edara <sedara@marvell.com>,
        Satananda Burla <sburla@marvell.com>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>
Subject: RE: [EXT] Re: [PATCH net-next v2 1/9] octeon_ep: defer probe if
 firmware not ready
Thread-Topic: [EXT] Re: [PATCH net-next v2 1/9] octeon_ep: defer probe if
 firmware not ready
Thread-Index: AQHZA/PZ82bWA4Tya0WGfhTwd2BjbK5XM0GAgABqSSA=
Date:   Wed, 30 Nov 2022 15:50:11 +0000
Message-ID: <BYAPR18MB242303FCE6E42D1467E4F420CC159@BYAPR18MB2423.namprd18.prod.outlook.com>
References: <20221129130933.25231-1-vburru@marvell.com>
 <20221129130933.25231-2-vburru@marvell.com> <Y4chWyR6qTlptkTE@unreal>
In-Reply-To: <Y4chWyR6qTlptkTE@unreal>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?Lc3vHUcReCZ8tPBQJ8PrMRTQdunVKHSZ4niXscWY+9Kwq/jUArImrt81WRJi?=
 =?us-ascii?Q?33nTrivM+Q4lbafSwMCowAqXvSXxB0X5e5rKzfKYSfRHitJNE/IeURoo5bKS?=
 =?us-ascii?Q?5fHoBBPvenroUtBu595oFLr1odsyMi7Oy9XwUofkWxcu2ZN9cQ4Ol51KXMf8?=
 =?us-ascii?Q?x2rb2NOPS+7DqbtwWBuz5WpXysZ6n2w2zkokKzFQQWiafTvTzWz9AEz+fz/w?=
 =?us-ascii?Q?mpRpAiAGNemGhii0erzw5vT+BsttU71aGWxMjR4UCdctUhBaO9c2o8PmwKFE?=
 =?us-ascii?Q?egdexD0mfG6pdWwLFNgqg1TBzGiMHc9QPORfzcnUU+V2hF0LGMfWGn4tVVM4?=
 =?us-ascii?Q?9H3QyImu4CPzCcLjv14fQn45CzsDuh1CAF21CA4ct4x1RfweHnNLumCzgwC+?=
 =?us-ascii?Q?qix/ADqUarY/FM+/mCgl5Kewhu59zbn/xIZNqbAGhfv2iUtFv16oW6PAPrCt?=
 =?us-ascii?Q?vQVlBK7xCbKiv8XMZ8Hyb0s3KuEdt4pCXwvq529O9TzSjP5CswEbkasToRF5?=
 =?us-ascii?Q?SdkpyeOtRhIvfNa7NP+++mLRAbaiWAcvCVWAbXF3jB57pGmvSvSd8ekEu8gK?=
 =?us-ascii?Q?sKP6VYRAJaCgKZStNy2gmgAnLdBR3gYpsx1FpJcbWQk3OR6zRmjK6taSW69c?=
 =?us-ascii?Q?7JDX50bgu5f835RxBFowfx2b41Yg4m3BKqvELWhw7ysuehfo8znXGPc8OBFP?=
 =?us-ascii?Q?aSZVso+XAGCUU+VpNSRBOgv+UdBNBN7Degv+ONDXXnCSyKwv2jLzc/ImqB9X?=
 =?us-ascii?Q?3jdjpTZxq8BYfUqTLa7/jYE5ztU6OJHW9IgimWi7bj75wZ88lF7RBKl1mCn2?=
 =?us-ascii?Q?Uz8uxNdYt5Z47VX4e5VZxSnRIwLtg+wCoEVNzfZiCmbAx1/eXqbehpU6LvTy?=
 =?us-ascii?Q?1Hqg1y/sGEzVi/+9K/NGcv2g1MRN4H45wDwjlQdoKmtidfeC/+V5INhd9ZiF?=
 =?us-ascii?Q?dDgkGvmeqxMDC5wfcmpulNgVn2JTJGT8D9leLCnQwrCCuwHgJOedw/EKvu+3?=
 =?us-ascii?Q?vVp6rS16zyRjUTFPaHyeeOWkw+v/8428yGlw5SAKhlMUKe59l2t0nyKqmR9j?=
 =?us-ascii?Q?89AUhUlROGytRm7wveJbkna6IQsr5aAGDpZUIZvaY1+J5t7nnkev9u+ERb32?=
 =?us-ascii?Q?Q1cxJgiq9qyTMC82B0vqsuPhVLt4WbLKDUnlI/nf/I9LMHLj5MgnkrCMUJKQ?=
 =?us-ascii?Q?urkNIHL3WMV7Jhfp9Y/M9JAMLO4qzjz63l6uQ+yN/wlrVYzJnrWc3iyzatDV?=
 =?us-ascii?Q?0sRhPcnRe3WxZ/b159XNPQEphK6X08vKwkFC/DNEE2VumU3boGkxrRfyxEtS?=
 =?us-ascii?Q?xAGheszJzjGIlEVj82PLI/7z/GkR0AwhIy9IPBBwEc/OywogBSZ2Boy9dPXr?=
 =?us-ascii?Q?S98hq1FC40x9rGrKclfvSSHfVuKiYcOuqIZIPrV6dztW9R/Q39pRAW4mJ/rt?=
 =?us-ascii?Q?jKRuUSvxRy9ecE5PaTdWKnMP409j2rApqr2X14arRouKKi0t/NL02AC9Fqtz?=
 =?us-ascii?Q?16fBheKyx0JKJoSzjztQhw7DKPWgRJRrxMxCBKQ1Z1dbQsyj83BDXFsnUkOg?=
 =?us-ascii?Q?e9haQhqu9gCCkTLRoqQ=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BYAPR18MB2423.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b94b3929-ccb7-42a0-6a37-08dad2ea908b
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Nov 2022 15:50:11.5269
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: UxT9vHOeCYq9wdMARTEHbjQoKrhHHucex4aPJ5zkLQG4qAO3hwY7wxyG0ONZgvw/pLlEeLyIlSUEQ5xl0wYOzw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR18MB3610
X-Proofpoint-GUID: ZClyKQvoCaf8D7af26UI9AJWmqQpgfZ2
X-Proofpoint-ORIG-GUID: ZClyKQvoCaf8D7af26UI9AJWmqQpgfZ2
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-11-30_04,2022-11-30_02,2022-06-22_01
X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> -----Original Message-----
> From: Leon Romanovsky <leon@kernel.org>
> Sent: Wednesday, November 30, 2022 1:25 AM
> To: Veerasenareddy Burru <vburru@marvell.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Liron Himi
> <lironh@marvell.com>; Abhijit Ayarekar <aayarekar@marvell.com>; Sathesh
> B Edara <sedara@marvell.com>; Satananda Burla <sburla@marvell.com>;
> linux-doc@vger.kernel.org; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> Paolo Abeni <pabeni@redhat.com>
> Subject: [EXT] Re: [PATCH net-next v2 1/9] octeon_ep: defer probe if
> firmware not ready
>=20
> External Email
>=20
> ----------------------------------------------------------------------
> On Tue, Nov 29, 2022 at 05:09:24AM -0800, Veerasenareddy Burru wrote:
> > Defer probe if firmware is not ready for device usage.
> >
> > Signed-off-by: Veerasenareddy Burru <vburru@marvell.com>
> > Signed-off-by: Abhijit Ayarekar <aayarekar@marvell.com>
> > Signed-off-by: Satananda Burla <sburla@marvell.com>
> > ---
> > v1 -> v2:
> >  * was scheduling workqueue task to wait for firmware ready,
> >    to probe/initialize the device.
> >  * now, removed the workqueue task; the probe returns EPROBE_DEFER,
> >    if firmware is not ready.
> >  * removed device status oct->status, as it is not required with the
> >    modified implementation.
> >
> >  .../ethernet/marvell/octeon_ep/octep_main.c   | 26
> +++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> > b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> > index 5a898fb88e37..aa7d0ced9807 100644
> > --- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> > +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> > @@ -1017,6 +1017,25 @@ static void octep_device_cleanup(struct
> octep_device *oct)
> >  	oct->conf =3D NULL;
> >  }
> >
> > +static u8 get_fw_ready_status(struct pci_dev *pdev)
>=20
> Please change this function to return bool, you are not interested in sta=
tus.
>=20
Yes, we can just return bool; Thanks for the suggestion. Will implement thi=
s.
> > +{
> > +	u32 pos =3D 0;
> > +	u16 vsec_id;
> > +	u8 status;
> > +
> > +	while ((pos =3D pci_find_next_ext_capability(pdev, pos,
> > +						   PCI_EXT_CAP_ID_VNDR))) {
> > +		pci_read_config_word(pdev, pos + 4, &vsec_id); #define
> > +FW_STATUS_VSEC_ID  0xA3
> > +		if (vsec_id =3D=3D FW_STATUS_VSEC_ID) {
>=20
> Success oriented flow, plase
> if (vsec_id !=3D FW_STATUS_VSEC_ID)
>  cotitnue;
>=20
> ....
>=20
Sure, will change this.
> > +			pci_read_config_byte(pdev, (pos + 8), &status);
> > +			dev_info(&pdev->dev, "Firmware ready %u\n",
> status);
> > +			return status;
> > +		}
> > +	}
> > +	return 0;
> > +}
> > +
> >  /**
> >   * octep_probe() - Octeon PCI device probe handler.
> >   *
> > @@ -1053,6 +1072,13 @@ static int octep_probe(struct pci_dev *pdev,
> const struct pci_device_id *ent)
> >  	pci_enable_pcie_error_reporting(pdev);
> >  	pci_set_master(pdev);
> >
> > +#define FW_STATUS_READY    1
> > +	if (get_fw_ready_status(pdev) !=3D FW_STATUS_READY) {
>=20
> No need to this new define if you change get_fw_ready_status() to return
> true/false.
We will change this to true/false.
>=20
> And I think that you can put this check earlier in octep_probe().
We will check and move this to earlier point in octep_probe().

Thanks
>=20
> Thanks
>=20
> > +		dev_notice(&pdev->dev, "Firmware not ready; defer
> probe.\n");
> > +		err =3D -EPROBE_DEFER;
> > +		goto err_alloc_netdev;
> > +	}
> > +
> >  	netdev =3D alloc_etherdev_mq(sizeof(struct octep_device),
> >  				   OCTEP_MAX_QUEUES);
> >  	if (!netdev) {
> > --
> > 2.36.0
> >