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:   <Tharunkumar.Pasumarthi@microchip.com>
To:     <andriy.shevchenko@linux.intel.com>,
        <Kumaravel.Thiagarajan@microchip.com>
CC:     <linux-kernel@vger.kernel.org>, <linux-serial@vger.kernel.org>,
        <gregkh@linuxfoundation.org>, <jirislaby@kernel.org>,
        <ilpo.jarvinen@linux.intel.com>, <macro@orcam.me.uk>,
        <cang1@live.co.uk>, <colin.i.king@gmail.com>,
        <phil.edworthy@renesas.com>, <biju.das.jz@bp.renesas.com>,
        <geert+renesas@glider.be>, <lukas@wunner.de>,
        <u.kleine-koenig@pengutronix.de>, <wander@redhat.com>,
        <etremblay@distech-controls.com>, <jk@ozlabs.org>,
        <UNGLinuxDriver@microchip.com>
Subject: RE: [PATCH v8 tty-next 2/4] serial: 8250_pci1xxxx: Add driver for
 quad-uart support
Thread-Topic: [PATCH v8 tty-next 2/4] serial: 8250_pci1xxxx: Add driver for
 quad-uart support
Thread-Index: AQHZDJkj7rnAVfXmX0mY3XTqVMobIK5nn64AgAI24uA=
Date:   Mon, 12 Dec 2022 07:16:55 +0000
Message-ID: <PH7PR11MB5958DC96634CFF28AFA8695D9BE29@PH7PR11MB5958.namprd11.prod.outlook.com>
References: <20221211014730.1233272-1-kumaravel.thiagarajan@microchip.com>
 <20221211014730.1233272-3-kumaravel.thiagarajan@microchip.com>
 <Y5T2ymgsCQhggtvz@smile.fi.intel.com>
In-Reply-To: <Y5T2ymgsCQhggtvz@smile.fi.intel.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?3GIdT8rVWUTZRGiivk1feLWra61YSZFNzMXOV/MQbxAX5mBVSfenwElsMn3N?=
 =?us-ascii?Q?1HOMauXiOHvhp134FoHU9ctfpgeYTI+kKZxwJZcGmMcM42cDUoyLqofk7X4v?=
 =?us-ascii?Q?wm6KyWSLZcipNbLVpuBJekqz30lMN2dPPFf29+btXW+aOJrC+p2KgFOfbvz1?=
 =?us-ascii?Q?gOe6BNVUExlLpY0qOExpSnUgID1BsKuCANTtdZOQY/UHiRLM8jy2oxGGQLwL?=
 =?us-ascii?Q?4n1xdydNdVGTzk2dD6Gq2iyy14AestdpHTenvArcATrmYu9qNZlyKJh/1vZt?=
 =?us-ascii?Q?fYOZsKQYJXNTIQbdiV6N0e/2vqayTJdjqVKjulizYbYtp37KhIi5ubRnwWbT?=
 =?us-ascii?Q?FHl7TPq5ceqeu6FXZBpLrZFNpxFfipqVHBs+HRIqWWnzkeKn+I4aQBLTl6QE?=
 =?us-ascii?Q?zwhRk86xv7Rs5n02zBOZQNRCe6Fxhj2OL2l3hLKOW9r60PviTWMELHsEE/PM?=
 =?us-ascii?Q?i873l357wMDhZh4vS22o5lPHjnFw73NBvJVvotZ4L4raIsrCw5AY5//sFlA7?=
 =?us-ascii?Q?25GdG1J4PGYSBffYWftwBu3yPKmRsmF8lArrnw988p/a4GSimiQ532GiKveB?=
 =?us-ascii?Q?fLMZy43q4bsovDTCxzXaPJlh5uh8wQcG7QFTebHFMULXT7+2mksTPfK1LoPb?=
 =?us-ascii?Q?UF8YgKjCrfyDPWW89tmDwX1Q5MKxOD1zQuDvsQLs0L3XHEHHoj7jHLEpOS2t?=
 =?us-ascii?Q?qFWrsv0C0Jecp9v5ucMrLFAhJvMFbTRONBdFYTRQmLa3CGCBrR2NUlSOHw5W?=
 =?us-ascii?Q?0GxYjsVnh33ZqqYr9LEkS2mI7XXQn6bAoQNJrlMXbFomaOBR1PW6+k+VhwNx?=
 =?us-ascii?Q?8xv1gizhaAcrmWT6++wKMZBA9bYPB7BD0XMWCj9Doh4e5/qf8oTac+6ueR2b?=
 =?us-ascii?Q?5bjFZgEz6cIYPVByZsJ0eZfidUwM7ahwr2GvPOllFC0zPjrZYQyIrIV4uX4u?=
 =?us-ascii?Q?AvuveosUZfaht0szPn2zPfk7CmbprIgHt+nnsO+P0LsXYrmZmb+8AYWu0LV1?=
 =?us-ascii?Q?HcC1EwVJCpkElZ6tN2g683bJIJAfk/ZxPXLCx3hFShXRq+NBhp0uBB3JVAD5?=
 =?us-ascii?Q?94jKuB1b053sGXxgPotlrJonjQo1EOTFcch0t3yTNVlSMRIrIp+QcpQtKYcl?=
 =?us-ascii?Q?Hyl0jh0SwRL/9r2mw4Oya0inCLXpoojKVYqnZPA6GkZs5uupOwpKTVV4M7DC?=
 =?us-ascii?Q?Xy/k4BsGiME4Fyxbaacb2yg8A+dnf2WM1aROcgB6nHbVR3An68SPRLozNNRh?=
 =?us-ascii?Q?1NzFLkqUrqmMWSas8vwm53LwL6Cr8dplnQbGTWlw3avwjgpmStmicrcixTk5?=
 =?us-ascii?Q?/7Elh4u6W+tRWmEXrrskDzuMlm5oYwkf+hhm9KARj7RSNcfE5oJPxKMV/5v4?=
 =?us-ascii?Q?P4lc9JMxKIJ0GLUHs7uIqLuD9jCwS+abFg6DdZ6wwWq7KJRT0QgjUj6yhauB?=
 =?us-ascii?Q?ViGvt+h+h9+ydSAstfq54PuQYElareqYSUg2GmXyEH/kkac0vyWOzGegXMrB?=
 =?us-ascii?Q?iImkzPKGEGIBnQvJWYJkzQ3YcbFIJkkaGBrAgFkI/MfyeustiKviGZEXs9z7?=
 =?us-ascii?Q?lukpwngYd0xYef6RMA8nv5m3dAwmOTZOjwexoThJTERuVjttnlyZhNkRAte1?=
 =?us-ascii?Q?k8jcC48vyHe9Oh3DePX5Jw4=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: 904ff5ff-18a5-4a87-0183-08dadc10d9df
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Dec 2022 07:16:55.8579
 (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: 6trd3wXVVmwYX7x21Jea0wO7f/M56yvLHlOGhsYUg3sWcR3Ueh8xrYUxlXRKocEEskkCMNg8kP3Zn65a5+6T71KcR2XdClPZVtxYxUtiC+n6DV91d4y1lpYKqNzCwwVq
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB5814
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS 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

> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Sent: Sunday, December 11, 2022 2:45 AM
> To: Kumaravel Thiagarajan - I21417
> <Kumaravel.Thiagarajan@microchip.com>
> Subject: Re: [PATCH v8 tty-next 2/4] serial: 8250_pci1xxxx: Add driver fo=
r
> quad-uart support
>=20
> > +             4,/* PCI1p3 */
> > +     };
>=20
> If you move this outside of the function you may use static_assert(), see
> below why.

If you move this outside of the function -> Do you suggest to move the arra=
y outside
the function and make it global?

>=20
>         if (subsys_dev <=3D ARRAY_SIZE(max_port))
>                 return max_port[subsys_dev];
>=20
> (in this case you can make sure it is the same as the above using
> static_assert(), so it won't compile otherwise)

I am not getting this. You suggest doing something like this:
static_assert(subsys_dev <=3D ARRAY_SIZE(max_port)) ?

> > +     priv->membase =3D pcim_iomap(pdev, 0, 0);
>=20
> As you said there will be no hardware that uses IO ports, why do you need
> pci_iomap()? I guess what you may use pci_ioremap_bar().
>=20
> > +     if (!priv->membase)
> > +             return -ENOMEM;

Okay, I will use pci_ioremap_bar(pdev, 0)

> > +     priv->nr =3D nr_ports;
> > +     pci_set_master(pdev);
> > +     max_vec_reqd =3D pci1xxxx_get_max_port(subsys_dev);
>=20
> The above needs a bit of reshuffling and perhaps a blank line or lines.
> Make it ordered and grouped more logically.

Okay. I will do something like this:
	pci_set_master(pdev);
	<NL>
	priv->pdev =3D pdev;
	priv->nr =3D nr_ports;
	<NL>
	subsys_dev =3D priv->pdev->subsystem_device;
	max_vec_reqd =3D pci1xxxx_get_max_port(subsys_dev);
	<NL>
	num_vectors =3D pci_alloc_irq_vectors(pdev, 1, max_vec_reqd, PCI_IRQ_ALL_T=
YPES);
	if (num_vectors < 0)
		return num_vectors;

> Can be simplified a bit by PCI_VDEVICE().

Okay.

Thanks,
Tharun Kumar P