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;
IronPort-SDR: a5mC0kUragGNpuFf/5Azb/PxDTqSu8qdfy8LB7BVmCP8Ts7x7cjt9o8U3KmoiqZkAhFIb3bF7k
 vlPwMVHWviordKGbb/RTOzQqOG96mlEtWAiSPIx9989Ik6TsM1z5YWUCz9N05dg+/eyNWWHZAa
 hXwHUSi7yXuom32Y3qTCj0221Zha0SbnJZ25a2exjVnj4+rmbDQFLJo/4qaPeETXxlYkKgcsG8
 kJNrqp0DlWO6+KopixQkigTglZsT3P1oiQ0Bka+an5kn1ohyRseK0uBICVKDwWCHZ8kqB8q1TU
 Wzg=
From:   Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
To:     Bjorn Helgaas <helgaas@kernel.org>
CC:     Puranjay Mohan <puranjay12@gmail.com>,
        "bjorn@helgaas.com" <bjorn@helgaas.com>,
        "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
        Damien Le Moal <Damien.LeMoal@wdc.com>,
        "axboe@kernel.dk" <axboe@kernel.dk>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] drivers: block: skd: remove skd_pci_info()
Thread-Topic: [PATCH] drivers: block: skd: remove skd_pci_info()
Thread-Index: AQHWz9z+X41O+6EQc0KJnRIciprIXQ==
Date:   Sat, 12 Dec 2020 01:59:51 +0000
Message-ID: <BYAPR04MB49650922C1631E8BEF425AA486C90@BYAPR04MB4965.namprd04.prod.outlook.com>
References: <20201211224151.GA113093@bjorn-Precision-5520>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: ca1db589-5da1-4edd-8c3e-08d89e419cbf
X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Dec 2020 01:59:51.1137
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 8wl8/rSPC6TvsSA81mXS6oT49nf7MbLExGXzB1YK2dKsF6wGAcRwNmShFvKmkUPfszDsdAouHd7tzBN4Ag3AW3/3GsTcB2XZQ4nmMl8tSNc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR04MB6422
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 12/11/20 14:41, Bjorn Helgaas wrote:=0A=
>> The skd driver prints unknown if the speed is not "2.5GT/s" or "5.0GT/s"=
.=0A=
>> __pcie_print_link_status()  prints "unknown" only if speed=0A=
>> value >=3D ARRAY_SIZE(speed_strings).=0A=
>>=0A=
>> If a buggy skd card returns value that is not !=3D ("2.5GT/s" or "5.0GT/=
s")=0A=
>> && value < ARRAY_SIZE(speed_strings) then it will not print the unknown =
but=0A=
>> the value from speed string array.=0A=
>>=0A=
>> Which breaks the current behavior. Please correct me if I'm wrong.=0A=
> I think you're right, but I don't think it actually *breaks* anything.=0A=
>=0A=
> For skd devices that work correctly, there should be no problem, and=0A=
> if there ever were an skd device that operated at a speed greater than=0A=
> 5GT/s, the PCI core would print that speed correctly instead of having=0A=
> the driver print "<unknown>".=0A=
That is the scenario I'm not aware why it prints unknown to start with=0A=
and I couldn't find any documentation also, that is why the concern.=0A=
> I don't think it's a good idea to have a driver artificially constrain=0A=
> the speed a device operates at.  And the existing code doesn't=0A=
> actually constrain anything; it only prints "<unknown>" if it doesn't=0A=
> recognize it.  The probe still succeeds.  I don't see much value in=0A=
> that "<unknown>".=0A=
>=0A=
> Or am I missing an actual problem this patch causes?=0A=
You are not, I'm just not sure without any documentation why does=0A=
it print "unknown" and I attributed that to probable firmware issue=0A=
(since we all knowhow creative firmware can get ;)).=0A=
=0A=
That makes it the problem with original code more so than with this patch.=
=0A=
In that case I was proposing just keep the original behavior.=0A=
=0A=
But maybe we should apply patch and if any user(s) comes up with the proble=
m=0A=
then we can deal with it.=0A=
=0A=
Whoever is going to apply they can add :-=0A=
=0A=
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>=0A=
=0A=
=0A=