[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <56037C09.5050201@linux.intel.com>
Date: Thu, 24 Sep 2015 12:28:57 +0800
From: Jiang Liu <jiang.liu@...ux.intel.com>
To: James Bottomley <James.Bottomley@...senPartnership.com>,
Arthur Marsh <arthur.marsh@...ernode.on.net>
Cc: Thomas Gleixner <tglx@...utronix.de>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Hannes Reinecke <hare@...e.de>,
Ballabio Dario <dario.ballabio@....com>,
Christoph Hellwig <hch@...radead.org>,
Dario Ballabio <ballabio_dario@....com>,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
linux-scsi@...r.kernel.org, x86@...nel.org
Subject: Re: [RFT v3] eata: Convert eata driver as normal PCI and platform
device drivers
On 2015/9/23 22:40, James Bottomley wrote:
> On Wed, 2015-09-23 at 20:14 +0930, Arthur Marsh wrote:
>>
>> Jiang Liu wrote on 23/09/15 14:54:
>>
>>> Hi Arthur,
>>> I have found the cause of the warning messages, it's caused
>>> by a flaw in the conversion. But according to my understanding,
>>> it isn't related to the kexec/kdump failure. Could you please help
>>> to test the attached new version?
>>> Thanks!
>>> Gerry
>>>
>>
>> Thanks, the patch worked, I could successfully unload and reload the
>> eata module, and perform a kexec reboot with the eata module loading
>> successfully afterwards.
>
> Great, so the bug was unconditionally unregistering the platform driver
> when it would fail to attach if none of the legacy IO ports were
> detected.
>
> I think the driver needs a bit of a tidy up. There's no need at all to
> use ida_get_simple(): the only reason for a dense array of numbers was
> for storing the hba private data in the array you got rid of; we can now
> simply use shost->host_no ... it's more useful anyway because the
> numbers match those SCSI is using.
>
> Also, if you insist on converting the printk's to dev warn, you no
> longer need to print out the driver name ... dev_printk already prints
> out the device and driver name as the prefix.
>
> The if (error == 0) is usually written as if (!error) but that's minor.
Hi James,
Thanks for review. How about the attached patch which addresses
the three suggestions from you?
Thanks!
Gerry
>
> Thanks for doing the conversion,
>
> James
>
>
View attachment "0001-.patch" of type "text/x-patch" (16418 bytes)
Powered by blists - more mailing lists