From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3620EC4338F for ; Thu, 19 Aug 2021 16:01:51 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D0FF560E90 for ; Thu, 19 Aug 2021 16:01:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org D0FF560E90 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=invisiblethingslab.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.168922.308486 (Exim 4.92) (envelope-from ) id 1mGkU8-0001FI-4g; Thu, 19 Aug 2021 16:01:40 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 168922.308486; Thu, 19 Aug 2021 16:01:40 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1mGkU8-0001FB-0x; Thu, 19 Aug 2021 16:01:40 +0000 Received: by outflank-mailman (input) for mailman id 168922; Thu, 19 Aug 2021 16:01:39 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1mGkU7-0001F5-AX for xen-devel@lists.xenproject.org; Thu, 19 Aug 2021 16:01:39 +0000 Received: from wout5-smtp.messagingengine.com (unknown [64.147.123.21]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id bbada5c5-0106-11ec-a63b-12813bfff9fa; Thu, 19 Aug 2021 16:01:38 +0000 (UTC) Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailout.west.internal (Postfix) with ESMTP id BE84632009A6; Thu, 19 Aug 2021 12:01:36 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Thu, 19 Aug 2021 12:01:37 -0400 Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 19 Aug 2021 12:01:34 -0400 (EDT) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: bbada5c5-0106-11ec-a63b-12813bfff9fa DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=Dy01h4 k1X8PMpBxnG2CJblDVIVGZEsmuDHXZ2qBVuSs=; b=FQ0ExT6vnjKDOALOlCK3B8 P8uLagat9mcURyBgbMyuwptevMYg42SBcpX1IkqnafWTzOIcUGJ6B+jq3L171Jd0 tX88SlD5cJ41MW8iNB8L1ylt4cd4tdbvsjOSgC7eESOlKFeVHl7E9ZIzkccdz0wo /arT8Huk2O2OBHvc9nMWJx6KFwhzc6dC/wI7DBwi1NAZliAVN/jzPV3386UTSHNz I+DKF+EBkvmyTm0ftiCQqPpDA2f0IfESCq7qTrX0IgNEm9G6n3si9/K68nriO827 xEZKliVlFj1DrWXooJR2lvFKN44G6rg1z5WxkSJ3bv5cm/VtQ3lMfEboI6zIwZQQ == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrleejgdelgecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvuffkfhggtggujgesghdtreertddtjeenucfhrhhomhepofgrrhgvkhcu ofgrrhgtiiihkhhofihskhhiqdfikphrvggtkhhiuceomhgrrhhmrghrvghksehinhhvih hsihgslhgvthhhihhnghhslhgrsgdrtghomheqnecuggftrfgrthhtvghrnhepteevffei gffhkefhgfegfeffhfegveeikeettdfhheevieehieeitddugeefteffnecuvehluhhsth gvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepmhgrrhhmrghrvghksehi nhhvihhsihgslhgvthhhihhnghhslhgrsgdrtghomh X-ME-Proxy: Date: Thu, 19 Aug 2021 18:01:31 +0200 From: Marek =?utf-8?Q?Marczykowski-G=C3=B3recki?= To: Jan Beulich Cc: Andrew Cooper , George Dunlap , Ian Jackson , Julien Grall , Stefano Stabellini , Wei Liu , xen-devel@lists.xenproject.org Subject: Re: [PATCH v2 2/2] ns16550: add Exar PCIe UART cards support Message-ID: References: <20210818121649.462413-1-marmarek@invisiblethingslab.com> <20210818121649.462413-2-marmarek@invisiblethingslab.com> <1be5e798-c3ee-afb0-3da1-7bf16d9f8c41@suse.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="aiWOjA+FJwpNhBhN" Content-Disposition: inline In-Reply-To: <1be5e798-c3ee-afb0-3da1-7bf16d9f8c41@suse.com> --aiWOjA+FJwpNhBhN Content-Type: text/plain; protected-headers=v1; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Date: Thu, 19 Aug 2021 18:01:31 +0200 From: Marek =?utf-8?Q?Marczykowski-G=C3=B3recki?= To: Jan Beulich Cc: Andrew Cooper , George Dunlap , Ian Jackson , Julien Grall , Stefano Stabellini , Wei Liu , xen-devel@lists.xenproject.org Subject: Re: [PATCH v2 2/2] ns16550: add Exar PCIe UART cards support On Thu, Aug 19, 2021 at 05:57:21PM +0200, Jan Beulich wrote: > On 18.08.2021 14:16, Marek Marczykowski-G=C3=B3recki wrote: > > @@ -169,6 +172,29 @@ static void handle_dw_usr_busy_quirk(struct ns1655= 0 *uart) > > } > > } > > =20 > > +static void enable_exar_enhanced_bits(struct ns16550 *uart) >=20 > Afaics the parameter can be pointer-to-const. >=20 > > +{ > > +#ifdef NS16550_PCI > > + if ( uart->bar && > > + pci_conf_read16(PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[2], > > + uart->ps_bdf[2]), PCI_VENDOR_ID) =3D=3D PCI_V= ENDOR_ID_EXAR ) > > + { > > + u8 devid =3D ns_read_reg(uart, UART_XR_DVID); > > + u8 efr; > > + /* > > + * Exar XR17V35x cards ignore setting MCR[2] (hardware flow co= ntrol) > > + * unless "Enhanced control bits" is enabled. > > + * The below checks for a 2, 4 or 8 port UART, following Linux= driver. > > + */ > > + if ( devid =3D=3D 0x82 || devid =3D=3D 0x84 || devid =3D=3D 0x= 88 ) { >=20 > Hmm, now I'm in trouble as to a question you did ask earlier (once > on irc and I think also once in email): You asked whether to use > the PCI device ID _or_ the DVID register. Now you're using both, > albeit in a way not really making the access here safe: You assume > that you can access the byte at offset 0x8d on all Exar devices. I > don't know whether there is anything written anywhere telling you > this is safe. With the earlier vendor/device ID match, it would feel > safer to me if you replaced vendor ID and DVID checks here by a > check of uart->param: While you must not deref that pointer, you can > still check that it points at one of the three new entries you add > to uart_param[]. Perhaps via "switch ( uart->param - uart_param )". Ok, indeed with checking just uart->param - uart_param, I can get rid of pci_conf_read here entirely. And so the #ifdef won't be necessary either. > Also there are a number of style nits: > - opening braces go on their own lines (except after "do"), > - blank lines are wanted between declarations and statements, > - we try to move away from u and want new code to use uint_t, > - with "devid" declared in the narrowest possible scope, please do > so also for "efr" (albeit this logic may get rearranged enough to > make this point moot). >=20 > Jan >=20 --=20 Best Regards, Marek Marczykowski-G=C3=B3recki Invisible Things Lab --aiWOjA+FJwpNhBhN Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAmEegFsACgkQ24/THMrX 1ywm0Af/XsYnQF1PBWjoYnDlovV1jGyIm7rjqn4tf33VdVmM62NdiHR8X/RL7D3D 1UC4iPTdBZt6cGsqralQ/EC4sXbuVVmZDcv82/v9F2duphCPqa/NvzLs0qC0BsMT YzcogfCRjj1o0q8XnhNj7wPDJiNHvDwnJhKmoN3Ux20MBwTIOlDzo3SCYssIwi06 0zMlmDJNZqErXutBEO9423E3KwXEJiDMlOjGzUYBNH3AQe7NChJ2qTP0BCoYLBVZ 91TGdOEtkOOUXZR0tdPozIxmi2RzH+RKsEQWe7sxYDFviSr+FFQMLuP9S/7/pf/C wZNRAtgmszq/LAGLUlNr63OaxYqxCQ== =5T3k -----END PGP SIGNATURE----- --aiWOjA+FJwpNhBhN--