* Re: [PATCH] Fix interface autodetection in legacy IDE driver (trial #2)
From: Bartlomiej Zolnierkiewicz @ 2016-12-27 10:08 UTC (permalink / raw)
To: David Miller; +Cc: lramos.prof, linux-ide, Borislav Petkov
In-Reply-To: <20161226.114724.642220063641900983.davem@davemloft.net>
Hi,
On Monday, December 26, 2016 11:47:24 AM David Miller wrote:
> From: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
> Date: Tue, 11 Oct 2016 22:12:45 -0300
>
> > This humble patch was sent one or two months before, and had no actions,
> > except for a colleague reply which friendly pointed out some formatting
> > problems (which were solved in a second message).
> >
> > It relates to an old code, the legacy IDE driver, but the bug it
> > addresses is real. The code, although rarely used, is
> > still there to be compiled if one chooses to do so (like me).
> >
> > Also, the fix has a very low risk of present collateral effects IMHO.
> > It is already compiled and tested in some embedded machines.
> >
> > So, again IMHO, it is worth be fixed.
> >
> > This email is a second trial with it. I hope it can help the one or two
> > guys out there which are still running the legacy IDE driver and
> > haven't noticed the former email.
> >
> > Best regards,
> >
> > Signed-off-by: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
>
> This bug was introduced by commit
> 20df429dd6671804999493baf2952f82582869fa ("ide-generic: handle probing
> of legacy io-ports v5") which seems poorly tested.
Please always cc: the original commit author.
> Applied and queued up for -stable, th anks.
For some reason I've never got the discussed patch from
linux-ide ML though I now have found in the patchwork:
https://patchwork.ozlabs.org/patch/680975/
The patch is incorrect. If you have PCI IDE devices (like in
the case described in the situation being "fixed" by the patch)
you should use the correct PCI IDE host driver for proper
operation and not ide-generic host driver (the latter still can
be used by using kernel parameters).
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [PATCH] Fix interface autodetection in legacy IDE driver (trial #2)
From: Bartlomiej Zolnierkiewicz @ 2016-12-27 16:06 UTC (permalink / raw)
To: David Miller; +Cc: lramos.prof, linux-ide, Borislav Petkov
In-Reply-To: <3010789.Juyl7C8p5j@amdc3058>
On Tuesday, December 27, 2016 11:08:24 AM Bartlomiej Zolnierkiewicz wrote:
>
> Hi,
>
> On Monday, December 26, 2016 11:47:24 AM David Miller wrote:
> > From: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
> > Date: Tue, 11 Oct 2016 22:12:45 -0300
> >
> > > This humble patch was sent one or two months before, and had no actions,
> > > except for a colleague reply which friendly pointed out some formatting
> > > problems (which were solved in a second message).
> > >
> > > It relates to an old code, the legacy IDE driver, but the bug it
> > > addresses is real. The code, although rarely used, is
> > > still there to be compiled if one chooses to do so (like me).
> > >
> > > Also, the fix has a very low risk of present collateral effects IMHO.
> > > It is already compiled and tested in some embedded machines.
> > >
> > > So, again IMHO, it is worth be fixed.
> > >
> > > This email is a second trial with it. I hope it can help the one or two
> > > guys out there which are still running the legacy IDE driver and
> > > haven't noticed the former email.
> > >
> > > Best regards,
> > >
> > > Signed-off-by: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
> >
> > This bug was introduced by commit
> > 20df429dd6671804999493baf2952f82582869fa ("ide-generic: handle probing
> > of legacy io-ports v5") which seems poorly tested.
>
> Please always cc: the original commit author.
>
> > Applied and queued up for -stable, th anks.
>
> For some reason I've never got the discussed patch from
> linux-ide ML though I now have found in the patchwork:
>
> https://patchwork.ozlabs.org/patch/680975/
>
> The patch is incorrect. If you have PCI IDE devices (like in
> the case described in the situation being "fixed" by the patch)
> you should use the correct PCI IDE host driver for proper
> operation and not ide-generic host driver (the latter still can
> be used by using kernel parameters).
Moreover this patch introduces a regression. In the situation
when there are no PCI IDE devices and the probing should be done
automatically (for the first two legacy IDE ports) it will be no
longer done.
Now back to the using correct PCI IDE host drivers - Luiz what
are the systems that you need this patch on? Could you please
get 'lspci -nn' command output from them?
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [PATCH] Fix interface autodetection in legacy IDE driver (trial #2)
From: David Miller @ 2016-12-27 16:41 UTC (permalink / raw)
To: b.zolnierkie; +Cc: lramos.prof, linux-ide, petkovbb
In-Reply-To: <6997612.Sd3D1QNmYZ@amdc3058>
From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Date: Tue, 27 Dec 2016 17:06:19 +0100
> On Tuesday, December 27, 2016 11:08:24 AM Bartlomiej Zolnierkiewicz wrote:
>>
>> Hi,
>>
>> On Monday, December 26, 2016 11:47:24 AM David Miller wrote:
>> > From: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
>> > Date: Tue, 11 Oct 2016 22:12:45 -0300
>> >
>> > > This humble patch was sent one or two months before, and had no actions,
>> > > except for a colleague reply which friendly pointed out some formatting
>> > > problems (which were solved in a second message).
>> > >
>> > > It relates to an old code, the legacy IDE driver, but the bug it
>> > > addresses is real. The code, although rarely used, is
>> > > still there to be compiled if one chooses to do so (like me).
>> > >
>> > > Also, the fix has a very low risk of present collateral effects IMHO.
>> > > It is already compiled and tested in some embedded machines.
>> > >
>> > > So, again IMHO, it is worth be fixed.
>> > >
>> > > This email is a second trial with it. I hope it can help the one or two
>> > > guys out there which are still running the legacy IDE driver and
>> > > haven't noticed the former email.
>> > >
>> > > Best regards,
>> > >
>> > > Signed-off-by: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
>> >
>> > This bug was introduced by commit
>> > 20df429dd6671804999493baf2952f82582869fa ("ide-generic: handle probing
>> > of legacy io-ports v5") which seems poorly tested.
>>
>> Please always cc: the original commit author.
>>
>> > Applied and queued up for -stable, th anks.
>>
>> For some reason I've never got the discussed patch from
>> linux-ide ML though I now have found in the patchwork:
>>
>> https://patchwork.ozlabs.org/patch/680975/
>>
>> The patch is incorrect. If you have PCI IDE devices (like in
>> the case described in the situation being "fixed" by the patch)
>> you should use the correct PCI IDE host driver for proper
>> operation and not ide-generic host driver (the latter still can
>> be used by using kernel parameters).
>
> Moreover this patch introduces a regression. In the situation
> when there are no PCI IDE devices and the probing should be done
> automatically (for the first two legacy IDE ports) it will be no
> longer done.
>
> Now back to the using correct PCI IDE host drivers - Luiz what
> are the systems that you need this patch on? Could you please
> get 'lspci -nn' command output from them?
The original code before the patch in question probed the interfaces
unconditionally, probe_mask was a static int set to "0x03".
Commit 20df429dd6671804999493baf2952f82582869fa changed the default
behavior, as well as adding a new module parameter whose behavior
makes no sense at all. Inverted bit logic? Give me a break.
Sorry, no, the fix is correct and I'm pushing it to Linus.
^ permalink raw reply
* LSF/MM 2017: Call for Proposals
From: Jeff Layton @ 2016-12-27 17:19 UTC (permalink / raw)
To: linux-block, linux-btrfs, linux-cifs, linux-ext4, linux-fsdevel,
linux-ide, linux-kernel, linux-mm, linux-nfs, linux-scsi,
xfs@oss.sgi.com, ceph-devel, linux-nvme
Cc: lsf-pc@lists.linux-foundation.org
We initially sent this pretty early this year, so this is a resend in
case anyone missed the first posting. The call for topics and attendance
requests is open until January 15th, 2017.
The original message follows:
----------------------8<------------------------
The annual Linux Storage, Filesystem and Memory Management (LSF/MM)
Summit for 2017 will be held on March 20th and 21st at the Hyatt
Cambridge, Cambridge, MA. LSF/MM is an invitation-only technical
workshop to map out improvements to the Linux storage, filesystem and
memory management subsystems that will make their way into the mainline
kernel within the coming years.
http://events.linuxfoundation.org/events/linux-storage-filesystem-and-mm-summit
Like last year, LSF/MM will be colocated with the Linux Foundation Vault
conference which takes place on March 22nd and 23rd in the same Venue.
For those that do not know, Vault is designed to be an event where open
source storage and filesystem practitioners meet storage implementors
and, as such, it would be of benefit for LSF/MM attendees to attend.
Unlike past years, Vault admission is not free for LSF/MM attendees this
year unless they're giving a talk. There is a discount for LSF/MM
attendees, however we would also like to encourage folks to submit talk
proposals to speak at the Vault conference.
http://events.linuxfoundation.org/events/vault
On behalf of the committee I am issuing a call for agenda proposals that
are suitable for cross-track discussion as well as technical subjects
for the breakout sessions.
If advance notice is required for visa applications then please point
that out in your proposal or request to attend, and submit the topic
as soon as possible.
1) Proposals for agenda topics should be sent before January 15th, 2016
to:
lsf-pc@lists.linux-foundation.org
and cc the Linux list or lists that are relevant for the topic in
question:
ATA: linux-ide@vger.kernel.org
Block: linux-block@vger.kernel.org
FS: linux-fsdevel@vger.kernel.org
MM: linux-mm@kvack.org
SCSI: linux-scsi@vger.kernel.org
NVMe: linux-nvme@lists.infradead.org
Please tag your proposal with [LSF/MM TOPIC] to make it easier to track.
In addition, please make sure to start a new thread for each topic
rather than following up to an existing one. Agenda topics and
attendees will be selected by the program committee, but the final
agenda will be formed by consensus of the attendees on the day.
2) Requests to attend the summit for those that are not proposing a
topic should be sent to:
lsf-pc@lists.linux-foundation.org
Please summarise what expertise you will bring to the meeting, and what
you would like to discuss. Please also tag your email with [LSF/MM
ATTEND] and send it as a new thread so there is less chance of it
getting lost.
We will try to cap attendance at around 25-30 per track to facilitate
discussions although the final numbers will depend on the room sizes at
the venue.
Brief presentations are allowed to guide discussion, but are strongly
discouraged. There will be no recording or audio bridge. However, we
expect that written minutes will be published as we did in previous
years:
2016: https://lwn.net/Articles/lsfmm2016/
2015: https://lwn.net/Articles/lsfmm2015/
2014: http://lwn.net/Articles/LSFMM2014/
2013: http://lwn.net/Articles/548089/
3) If you have feedback on last year's meeting that we can use to
improve this year's, please also send that to:
lsf-pc@lists.linux-foundation.org
Thank you on behalf of the program committee:
Storage:
James Bottomley
Martin K. Petersen (track chair)
Sagi Grimberg
Filesystems:
Anna Schumaker
Chris Mason
Eric Sandeen
Jan Kara
Jeff Layton (summit chair)
Josef Bacik (track chair)
Trond Myklebust
MM:
Johannes Weiner
Rik van Riel (track chair)
--
Jeff Layton <jlayton@poochiereds.net>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] Fix interface autodetection in legacy IDE driver (trial #2)
From: Bartlomiej Zolnierkiewicz @ 2016-12-27 17:25 UTC (permalink / raw)
To: David Miller; +Cc: lramos.prof, linux-ide, petkovbb
In-Reply-To: <20161227.114112.712597535585873594.davem@davemloft.net>
Hi,
On Tuesday, December 27, 2016 11:41:12 AM David Miller wrote:
> From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Date: Tue, 27 Dec 2016 17:06:19 +0100
>
> > On Tuesday, December 27, 2016 11:08:24 AM Bartlomiej Zolnierkiewicz wrote:
> >>
> >> Hi,
> >>
> >> On Monday, December 26, 2016 11:47:24 AM David Miller wrote:
> >> > From: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
> >> > Date: Tue, 11 Oct 2016 22:12:45 -0300
> >> >
> >> > > This humble patch was sent one or two months before, and had no actions,
> >> > > except for a colleague reply which friendly pointed out some formatting
> >> > > problems (which were solved in a second message).
> >> > >
> >> > > It relates to an old code, the legacy IDE driver, but the bug it
> >> > > addresses is real. The code, although rarely used, is
> >> > > still there to be compiled if one chooses to do so (like me).
> >> > >
> >> > > Also, the fix has a very low risk of present collateral effects IMHO.
> >> > > It is already compiled and tested in some embedded machines.
> >> > >
> >> > > So, again IMHO, it is worth be fixed.
> >> > >
> >> > > This email is a second trial with it. I hope it can help the one or two
> >> > > guys out there which are still running the legacy IDE driver and
> >> > > haven't noticed the former email.
> >> > >
> >> > > Best regards,
> >> > >
> >> > > Signed-off-by: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
> >> >
> >> > This bug was introduced by commit
> >> > 20df429dd6671804999493baf2952f82582869fa ("ide-generic: handle probing
> >> > of legacy io-ports v5") which seems poorly tested.
> >>
> >> Please always cc: the original commit author.
> >>
> >> > Applied and queued up for -stable, th anks.
> >>
> >> For some reason I've never got the discussed patch from
> >> linux-ide ML though I now have found in the patchwork:
> >>
> >> https://patchwork.ozlabs.org/patch/680975/
> >>
> >> The patch is incorrect. If you have PCI IDE devices (like in
> >> the case described in the situation being "fixed" by the patch)
> >> you should use the correct PCI IDE host driver for proper
> >> operation and not ide-generic host driver (the latter still can
> >> be used by using kernel parameters).
> >
> > Moreover this patch introduces a regression. In the situation
> > when there are no PCI IDE devices and the probing should be done
> > automatically (for the first two legacy IDE ports) it will be no
> > longer done.
> >
> > Now back to the using correct PCI IDE host drivers - Luiz what
> > are the systems that you need this patch on? Could you please
> > get 'lspci -nn' command output from them?
>
> The original code before the patch in question probed the interfaces
> unconditionally, probe_mask was a static int set to "0x03".
>
> Commit 20df429dd6671804999493baf2952f82582869fa changed the default
> behavior, as well as adding a new module parameter whose behavior
> makes no sense at all. Inverted bit logic? Give me a break.
>
> Sorry, no, the fix is correct and I'm pushing it to Linus.
The "fix" is not correct and is not needed. 99% of users of ide-generic
used it by mistake and should have used the designated host drivers for
their hardware or PCI IDE generic host driver (not ide-generic one).
Alan Cox did the work on fixing this for his pata_legacy libata host
driver and later Borislav back-ported needed changes to ide-generic host
driver in commit 20df429dd6671804999493baf2952f82582869fa (in *2008*).
Also the "fix" is not a revert but a new patch which introduces a real
regression described by me in the previous mail.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [PATCH] Fix interface autodetection in legacy IDE driver (trial #2)
From: David Miller @ 2016-12-28 0:38 UTC (permalink / raw)
To: lramos.prof; +Cc: b.zolnierkie, linux-ide, petkovbb
In-Reply-To: <20161228003335.GA3720@giustizia>
From: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
Date: Tue, 27 Dec 2016 22:33:35 -0200
> So, ide_generic_init() should test *primary for 1 in the case of an
> existing IDE primary resource, and *secondary for 1 in the case of a
> secondary IDE resource.
>
> Unpatched code checks both for zero in order to set the proper bits in
> probe_mask, and IMHO this is reversed logic.
Right, and I can't see how this is intentional at all.
^ permalink raw reply
* Re: [PATCH] Fix interface autodetection in legacy IDE driver (trial #2)
From: Luiz Carlos Ramos @ 2016-12-28 0:33 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: David Miller, linux-ide, petkovbb
In-Reply-To: <1680456.bXBqjl9psU@amdc3058>
Hi,
I really don't know if my opinions will be well received, but here we go.
Please apologize me if you feel me rude, but I'm just trying to explain
the problem the way I'm seeing it, and also to understand what you are
saying the best I can.
On Tue, Dec 27, 2016 at 06:25:18PM +0100, Bartlomiej Zolnierkiewicz wrote:
>
> Hi,
>
> On Tuesday, December 27, 2016 11:41:12 AM David Miller wrote:
> > From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > Date: Tue, 27 Dec 2016 17:06:19 +0100
> >
> > > On Tuesday, December 27, 2016 11:08:24 AM Bartlomiej Zolnierkiewicz wrote:
> > >>
> > >> Hi,
> > >>
> > >> On Monday, December 26, 2016 11:47:24 AM David Miller wrote:
> > >> > From: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
> > >> > Date: Tue, 11 Oct 2016 22:12:45 -0300
> > >> >
> > >> > > This humble patch was sent one or two months before, and had no actions,
> > >> > > except for a colleague reply which friendly pointed out some formatting
> > >> > > problems (which were solved in a second message).
> > >> > >
> > >> > > It relates to an old code, the legacy IDE driver, but the bug it
> > >> > > addresses is real. The code, although rarely used, is
> > >> > > still there to be compiled if one chooses to do so (like me).
> > >> > >
> > >> > > Also, the fix has a very low risk of present collateral effects IMHO.
> > >> > > It is already compiled and tested in some embedded machines.
> > >> > >
> > >> > > So, again IMHO, it is worth be fixed.
> > >> > >
> > >> > > This email is a second trial with it. I hope it can help the one or two
> > >> > > guys out there which are still running the legacy IDE driver and
> > >> > > haven't noticed the former email.
> > >> > >
> > >> > > Best regards,
> > >> > >
> > >> > > Signed-off-by: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
> > >> >
> > >> > This bug was introduced by commit
> > >> > 20df429dd6671804999493baf2952f82582869fa ("ide-generic: handle probing
> > >> > of legacy io-ports v5") which seems poorly tested.
> > >>
> > >> Please always cc: the original commit author.
> > >>
> > >> > Applied and queued up for -stable, th anks.
> > >>
> > >> For some reason I've never got the discussed patch from
> > >> linux-ide ML though I now have found in the patchwork:
> > >>
> > >> https://patchwork.ozlabs.org/patch/680975/
> > >>
> > >> The patch is incorrect. If you have PCI IDE devices (like in
I think all of you have checked the code, but I will try to describe
what I'm seeing.
The routine ide_generic_check_pci_legacy_iobases() returns *primary
equal to 1 if some PCI IDE resource is found at 0x1f0, and *secondary
equal to 1 if some PCI IDE resource is found at 0x1e0. The same
routine doesn't change neither *primary not *secondary if nothing is
found. The only caller - ide_generic_init() - initializes both to zero
before the call.
So, ide_generic_init() should test *primary for 1 in the case of an
existing IDE primary resource, and *secondary for 1 in the case of a
secondary IDE resource.
Unpatched code checks both for zero in order to set the proper bits in
probe_mask, and IMHO this is reversed logic.
There are two ways of fixing this. One is to test *primary and
*secondary for "not zero" in ide_generic_init(), as proposed. The other
way is to change ide_generic_check_pci_legacy_iobases() in order to
return *primary or *secondary with zero in case of success when
searching for PCI resources - along with setting them to "not zero"
before checking.
> > >> The patch is incorrect. If you have PCI IDE devices (like in
> > >> the case described in the situation being "fixed" by the patch)
> > >> you should use the correct PCI IDE host driver for proper
> > >> operation and not ide-generic host driver (the latter still can
> > >> be used by using kernel parameters).
> > >
Please elaborate more on this; I really haven't understood. It seems that
you're telling about PCI host driver code, which I haven't studied. I
haven't caught the whole picture.
In my case, there are two IDE interfaces, which are at the common
addresses 0x1f0 and 0x1e0 (PCI also tells me that), and the chipset is
a CS5536. The previous kernel used in the system (2.6.16) worked with
ide-generic, and the one I'm testing now (3.18.x) doesn't bring any of
the IDE interfaces up.
You're right in the sense I haven't compiled the CS5536 legacy host
driver code; so, I can't tell about how it works.
Of course there are many reasons to use the correct PCI IDE host
driver, but at the end, it's up to the user to choose which one he/she will
use, and ideally any of them should work.
> > > Moreover this patch introduces a regression. In the situation
> > > when there are no PCI IDE devices and the probing should be done
> > > automatically (for the first two legacy IDE ports) it will be no
> > > longer done.
> > >
Again, please elaborate more on this.
If there are no PCI IDE devices, well, there are no PCI IDE devices.
Nothing is supposed to be found. Anyway, it seems they will be searched
for, in ide_generic_check_pci_legacy_iobases(). Or I missed something
important.
> > > Now back to the using correct PCI IDE host drivers - Luiz what
> > > are the systems that you need this patch on? Could you please
> > > get 'lspci -nn' command output from them?
> >
> > The original code before the patch in question probed the interfaces
> > unconditionally, probe_mask was a static int set to "0x03".
> >
> > Commit 20df429dd6671804999493baf2952f82582869fa changed the default
> > behavior, as well as adding a new module parameter whose behavior
> > makes no sense at all. Inverted bit logic? Give me a break.
> >
> > Sorry, no, the fix is correct and I'm pushing it to Linus.
>
> The "fix" is not correct and is not needed. 99% of users of ide-generic
> used it by mistake and should have used the designated host drivers for
> their hardware or PCI IDE generic host driver (not ide-generic one).
My system didn't work without this patch. I agree I could use the
designated host driver, but it seems a little strong to say one _should_
use it.
A newer version of the system uses libata, and yes, it works.
>
> Alan Cox did the work on fixing this for his pata_legacy libata host
> driver and later Borislav back-ported needed changes to ide-generic host
> driver in commit 20df429dd6671804999493baf2952f82582869fa (in *2008*).
>
> Also the "fix" is not a revert but a new patch which introduces a real
> regression described by me in the previous mail.
>
As said above, it is a regression considering the versions from 2006. I
agree that for someone who used the drivers from 2009 and after, it
_may_ be a regression if she uses to bring all up with probe_mask=0.
IMHO, it's the case where a bug later becomes a feature.
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>
Best regards,
Luiz Carlos Ramos
São Paulo - Brazil
^ permalink raw reply
* Re: [PATCH] Fix interface autodetection in legacy IDE driver (trial #2)
From: Bartlomiej Zolnierkiewicz @ 2016-12-28 11:16 UTC (permalink / raw)
To: David Miller; +Cc: lramos.prof, linux-ide, petkovbb
In-Reply-To: <20161227.193818.1314136105272589794.davem@davemloft.net>
Hi,
On Tuesday, December 27, 2016 07:38:18 PM David Miller wrote:
> From: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
> Date: Tue, 27 Dec 2016 22:33:35 -0200
>
> > So, ide_generic_init() should test *primary for 1 in the case of an
> > existing IDE primary resource, and *secondary for 1 in the case of a
> > secondary IDE resource.
> >
> > Unpatched code checks both for zero in order to set the proper bits in
> > probe_mask, and IMHO this is reversed logic.
>
> Right, and I can't see how this is intentional at all.
If PCI IDE resource is found we shouldn't do the probe automatically
as ide-generic is not the proper driver to run the hardware. IOW we
only want to do automatic probe if no PCI IDE resources are found (if
primary/secondary == 0). In such case we are most likely running on
pre-PCI system and should be probing for legacy ISA IDE ports. Please
note they are using the same I/O resources as PCI IDE ones (!).
[ Please see the reply to Luiz for the full explanation. ]
I know that you're busy and don't have time for in-depth analysis
of non-obvious IDE patches. I would like to help and (co-)maintain
the IDE subsystem (deep maintenance mode policy would of course be
preserved).
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [PATCH] Fix interface autodetection in legacy IDE driver (trial #2)
From: Bartlomiej Zolnierkiewicz @ 2016-12-28 11:10 UTC (permalink / raw)
To: Luiz Carlos Ramos; +Cc: David Miller, linux-ide, petkovbb
In-Reply-To: <20161228003335.GA3720@giustizia>
Hi,
On Tuesday, December 27, 2016 10:33:35 PM Luiz Carlos Ramos wrote:
> Hi,
>
> I really don't know if my opinions will be well received, but here we go.
>
> Please apologize me if you feel me rude, but I'm just trying to explain
> the problem the way I'm seeing it, and also to understand what you are
> saying the best I can.
>
> On Tue, Dec 27, 2016 at 06:25:18PM +0100, Bartlomiej Zolnierkiewicz wrote:
> >
> > Hi,
> >
> > On Tuesday, December 27, 2016 11:41:12 AM David Miller wrote:
> > > From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > > Date: Tue, 27 Dec 2016 17:06:19 +0100
> > >
> > > > On Tuesday, December 27, 2016 11:08:24 AM Bartlomiej Zolnierkiewicz wrote:
> > > >>
> > > >> Hi,
> > > >>
> > > >> On Monday, December 26, 2016 11:47:24 AM David Miller wrote:
> > > >> > From: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
> > > >> > Date: Tue, 11 Oct 2016 22:12:45 -0300
> > > >> >
> > > >> > > This humble patch was sent one or two months before, and had no actions,
> > > >> > > except for a colleague reply which friendly pointed out some formatting
> > > >> > > problems (which were solved in a second message).
> > > >> > >
> > > >> > > It relates to an old code, the legacy IDE driver, but the bug it
> > > >> > > addresses is real. The code, although rarely used, is
> > > >> > > still there to be compiled if one chooses to do so (like me).
> > > >> > >
> > > >> > > Also, the fix has a very low risk of present collateral effects IMHO.
> > > >> > > It is already compiled and tested in some embedded machines.
> > > >> > >
> > > >> > > So, again IMHO, it is worth be fixed.
> > > >> > >
> > > >> > > This email is a second trial with it. I hope it can help the one or two
> > > >> > > guys out there which are still running the legacy IDE driver and
> > > >> > > haven't noticed the former email.
> > > >> > >
> > > >> > > Best regards,
> > > >> > >
> > > >> > > Signed-off-by: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
> > > >> >
> > > >> > This bug was introduced by commit
> > > >> > 20df429dd6671804999493baf2952f82582869fa ("ide-generic: handle probing
> > > >> > of legacy io-ports v5") which seems poorly tested.
> > > >>
> > > >> Please always cc: the original commit author.
> > > >>
> > > >> > Applied and queued up for -stable, th anks.
> > > >>
> > > >> For some reason I've never got the discussed patch from
> > > >> linux-ide ML though I now have found in the patchwork:
> > > >>
> > > >> https://patchwork.ozlabs.org/patch/680975/
> > > >>
> > > >> The patch is incorrect. If you have PCI IDE devices (like in
>
> I think all of you have checked the code, but I will try to describe
> what I'm seeing.
>
> The routine ide_generic_check_pci_legacy_iobases() returns *primary
> equal to 1 if some PCI IDE resource is found at 0x1f0, and *secondary
> equal to 1 if some PCI IDE resource is found at 0x1e0. The same
> routine doesn't change neither *primary not *secondary if nothing is
> found. The only caller - ide_generic_init() - initializes both to zero
> before the call.
>
> So, ide_generic_init() should test *primary for 1 in the case of an
> existing IDE primary resource, and *secondary for 1 in the case of a
> secondary IDE resource.
>
> Unpatched code checks both for zero in order to set the proper bits in
> probe_mask, and IMHO this is reversed logic.
Unpatched code is correct.
If PCI IDE resource is found we shouldn't do the probe automatically
as ide-generic is not the proper driver to run the hardware. IOW we
only want to do automatic probe if no PCI IDE resources are found (if
primary/secondary == 0). In such case we are most likely running on
pre-PCI system and should be probing for legacy ISA IDE ports. Please
note they are are using the same I/O resources as PCI IDE ones (!).
> There are two ways of fixing this. One is to test *primary and
> *secondary for "not zero" in ide_generic_init(), as proposed. The other
> way is to change ide_generic_check_pci_legacy_iobases() in order to
> return *primary or *secondary with zero in case of success when
> searching for PCI resources - along with setting them to "not zero"
> before checking.
No need to change anything. Original code is correct.
> > > >> The patch is incorrect. If you have PCI IDE devices (like in
> > > >> the case described in the situation being "fixed" by the patch)
> > > >> you should use the correct PCI IDE host driver for proper
> > > >> operation and not ide-generic host driver (the latter still can
> > > >> be used by using kernel parameters).
> > > >
>
> Please elaborate more on this; I really haven't understood. It seems that
> you're telling about PCI host driver code, which I haven't studied. I
> haven't caught the whole picture.
>
> In my case, there are two IDE interfaces, which are at the common
> addresses 0x1f0 and 0x1e0 (PCI also tells me that), and the chipset is
> a CS5536. The previous kernel used in the system (2.6.16) worked with
> ide-generic, and the one I'm testing now (3.18.x) doesn't bring any of
> the IDE interfaces up.
>
> You're right in the sense I haven't compiled the CS5536 legacy host
> driver code; so, I can't tell about how it works.
You should be using CS5336 PCI IDE host driver (drivers/ide/cs5536.c
enabled by CONFIG_BLK_DEV_CS5536 config option) for optimal performance
and proper operations.
CS5536 PCI IDE host driver was added in kernel v2.6.29.
> Of course there are many reasons to use the correct PCI IDE host
> driver, but at the end, it's up to the user to choose which one he/she will
> use, and ideally any of them should work.
It happens to "work" on your system because CS5536 is relatively
simple and non-buggy PCI IDE chipset.
In case of other PCI IDE chipsets you may not only be missing
performance or some functionality - on some more quirky chipsets
you may be also affecting data integrity (!).
Anyway you still can use ide-generic by using kernel parameters
(just add "ide_generic.probe_mask=0x03" to your kernel command line).
I agree that the ide-generic could be more verbose and print more
information in case of skipping the probe (patches are welcomed).
> > > > Moreover this patch introduces a regression. In the situation
> > > > when there are no PCI IDE devices and the probing should be done
> > > > automatically (for the first two legacy IDE ports) it will be no
> > > > longer done.
> > > >
>
> Again, please elaborate more on this.
>
> If there are no PCI IDE devices, well, there are no PCI IDE devices.
> Nothing is supposed to be found. Anyway, it seems they will be searched
> for, in ide_generic_check_pci_legacy_iobases(). Or I missed something
> important.
In case there are no PCI IDE devices we most likely are running on
pre-PCI system and should be probing for legacy ISA IDE ports (please
note they are are using the same I/O resources),
> > > > Now back to the using correct PCI IDE host drivers - Luiz what
> > > > are the systems that you need this patch on? Could you please
> > > > get 'lspci -nn' command output from them?
> > >
> > > The original code before the patch in question probed the interfaces
> > > unconditionally, probe_mask was a static int set to "0x03".
> > >
> > > Commit 20df429dd6671804999493baf2952f82582869fa changed the default
> > > behavior, as well as adding a new module parameter whose behavior
> > > makes no sense at all. Inverted bit logic? Give me a break.
> > >
> > > Sorry, no, the fix is correct and I'm pushing it to Linus.
> >
> > The "fix" is not correct and is not needed. 99% of users of ide-generic
> > used it by mistake and should have used the designated host drivers for
> > their hardware or PCI IDE generic host driver (not ide-generic one).
>
> My system didn't work without this patch. I agree I could use the
> designated host driver, but it seems a little strong to say one _should_
> use it.
Sorry but one should really use the designated drivers and we don't
want to see any bugreports from using ide-generic on hardware that
have designated drivers.
> A newer version of the system uses libata, and yes, it works.
In libata there is the same policy as in IDE - pata_legacy host driver
(libata's equivalent of ide-generic host driver) was the first one
using the new policy, it was later back-ported to ide-generic.
> > Alan Cox did the work on fixing this for his pata_legacy libata host
> > driver and later Borislav back-ported needed changes to ide-generic host
> > driver in commit 20df429dd6671804999493baf2952f82582869fa (in *2008*).
> >
> > Also the "fix" is not a revert but a new patch which introduces a real
> > regression described by me in the previous mail.
> >
>
> As said above, it is a regression considering the versions from 2006. I
> agree that for someone who used the drivers from 2009 and after, it
> _may_ be a regression if she uses to bring all up with probe_mask=0.
The regression I was talking about is in the proposed patch.
By simply reversing the logic ISA IDE ports will no longer be probed
automatically on non-PCI systems.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> IMHO, it's the case where a bug later becomes a feature.
>
> > Best regards,
> > --
> > Bartlomiej Zolnierkiewicz
> > Samsung R&D Institute Poland
> > Samsung Electronics
> >
>
> Best regards,
>
> Luiz Carlos Ramos
> São Paulo - Brazil
^ permalink raw reply
* (unknown),
From: beautyink @ 2016-12-29 22:55 UTC (permalink / raw)
To: linux-ide
[-- Attachment #1: 02689684_linux-ide.zip --]
[-- Type: application/zip, Size: 41489 bytes --]
^ permalink raw reply
* Re: [PATCH] Fix interface autodetection in legacy IDE driver (trial #2)
From: Luiz Carlos Ramos @ 2016-12-30 0:52 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: David Miller, linux-ide, petkovbb
In-Reply-To: <1544514.cVcK9YN4LR@amdc3058>
Hi,
I think I could catch the whole picture after your explanation. I'll try
to summarize all the points discussed below in the body.
My suggestion now is to drop the current (proposed) patch and substitute
it for a pure "documentational" patch, as it seems for me that a
programmer new to that subsystem - as the case of myself - tends to find
ide-generic.c buggy, if analyzed out of the whole context. Some more
inline documentation may help all of us IMHO.
Of course improving the printed messages - as suggested - can help as well.
I'd like to hear from David (cc'ed) about this suggestion. What would you David
suggest?
Anyway, there is a "break of the contract" with the old patch which
introduced the search for the IDE interfaces. Before, it was not necessary
to specify probe_mask=0x03 or like to make ide-generic find something in
I/O addresses 0x1f0 and 0x170 (I wrongly wrote 0x1e0 in the former emails;
please apologize me for that mistake). But this contract break seems to be
justifiable on the grounds that - by a design decision - users "are encouraged"
to use the designated drivers as a basic axiom. I suppose (or I hope) this
was discussed at that time and this decision was taken consciously.
On Wed, Dec 28, 2016 at 12:10:16PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > > >> > > This humble patch was sent one or two months before, and had no actions,
> > > > >> > > except for a colleague reply which friendly pointed out some formatting
> > > > >> > > problems (which were solved in a second message).
> > > > >> > >
> > > > >> > > It relates to an old code, the legacy IDE driver, but the bug it
> > > > >> > > addresses is real. The code, although rarely used, is
> > > > >> > > still there to be compiled if one chooses to do so (like me).
> > > > >> > >
> > > > >> > > Also, the fix has a very low risk of present collateral effects IMHO.
> > > > >> > > It is already compiled and tested in some embedded machines.
> > > > >> > >
> > > > >> > > So, again IMHO, it is worth be fixed.
> > > > >> > >
> > > > >> > > This email is a second trial with it. I hope it can help the one or two
> > > > >> > > guys out there which are still running the legacy IDE driver and
> > > > >> > > haven't noticed the former email.
> > > > >> > >
> > > > >> > > Best regards,
> > > > >> > >
> > > > >> > > Signed-off-by: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
> > > > >> >
> > > > >> > This bug was introduced by commit
> > > > >> > 20df429dd6671804999493baf2952f82582869fa ("ide-generic: handle probing
> > > > >> > of legacy io-ports v5") which seems poorly tested.
> > > > >>
> > > > >> Please always cc: the original commit author.
> > > > >>
> > > > >> > Applied and queued up for -stable, th anks.
> > > > >>
> > > > >> For some reason I've never got the discussed patch from
> > > > >> linux-ide ML though I now have found in the patchwork:
> > > > >>
> > > > >> https://patchwork.ozlabs.org/patch/680975/
> > > > >>
> > > > >> The patch is incorrect. If you have PCI IDE devices (like in
> >
> > I think all of you have checked the code, but I will try to describe
> > what I'm seeing.
> >
> > The routine ide_generic_check_pci_legacy_iobases() returns *primary
> > equal to 1 if some PCI IDE resource is found at 0x1f0, and *secondary
> > equal to 1 if some PCI IDE resource is found at 0x1e0. The same
> > routine doesn't change neither *primary not *secondary if nothing is
> > found. The only caller - ide_generic_init() - initializes both to zero
> > before the call.
> >
> > So, ide_generic_init() should test *primary for 1 in the case of an
> > existing IDE primary resource, and *secondary for 1 in the case of a
> > secondary IDE resource.
> >
> > Unpatched code checks both for zero in order to set the proper bits in
> > probe_mask, and IMHO this is reversed logic.
>
> Unpatched code is correct.
>
> If PCI IDE resource is found we shouldn't do the probe automatically
> as ide-generic is not the proper driver to run the hardware. IOW we
> only want to do automatic probe if no PCI IDE resources are found (if
> primary/secondary == 0). In such case we are most likely running on
> pre-PCI system and should be probing for legacy ISA IDE ports. Please
> note they are are using the same I/O resources as PCI IDE ones (!).
>
Understood. ide-generic should not use devices which have a PCI
interface, unless the user tells it to do it (using probe_mask!=0).
The basic assumption - as stated I think in your first reply - is that
by design, users should use the driver more suitable for the chipset, if
it has a PCI interface, and not ide-generic.
I think this assumption should be highlighted as an axiom, or a design
decision. Otherwise, one could argue in favor of enabling ide-generic as a first
option for handling the IDE interface, be that PCI or IDE.
Also is supposed that this decision is not subject to discussion (now).
> > > > >> The patch is incorrect. If you have PCI IDE devices (like in
> > > > >> the case described in the situation being "fixed" by the patch)
> > > > >> you should use the correct PCI IDE host driver for proper
> > > > >> operation and not ide-generic host driver (the latter still can
> > > > >> be used by using kernel parameters).
> > > > >
> >
> > Please elaborate more on this; I really haven't understood. It seems that
> > you're telling about PCI host driver code, which I haven't studied. I
> > haven't caught the whole picture.
> >
> > In my case, there are two IDE interfaces, which are at the common
> > addresses 0x1f0 and 0x1e0 (PCI also tells me that), and the chipset is
> > a CS5536. The previous kernel used in the system (2.6.16) worked with
> > ide-generic, and the one I'm testing now (3.18.x) doesn't bring any of
> > the IDE interfaces up.
> >
> > You're right in the sense I haven't compiled the CS5536 legacy host
> > driver code; so, I can't tell about how it works.
>
> You should be using CS5336 PCI IDE host driver (drivers/ide/cs5536.c
> enabled by CONFIG_BLK_DEV_CS5536 config option) for optimal performance
> and proper operations.
>
> CS5536 PCI IDE host driver was added in kernel v2.6.29.
This can be one reason to not have CS5536 driver used in the first
version of the system (kernel 2.6.16).
>
> > Of course there are many reasons to use the correct PCI IDE host
> > driver, but at the end, it's up to the user to choose which one he/she will
> > use, and ideally any of them should work.
>
> It happens to "work" on your system because CS5536 is relatively
> simple and non-buggy PCI IDE chipset.
>
> In case of other PCI IDE chipsets you may not only be missing
> performance or some functionality - on some more quirky chipsets
> you may be also affecting data integrity (!).
>
> Anyway you still can use ide-generic by using kernel parameters
> (just add "ide_generic.probe_mask=0x03" to your kernel command line).
>
As said before, this is a contract break between versions from, let's
say, 2006 and today's.
Anyway, there's no way to avoid it, it we take the assumption above
(designated drivers prior to ide-generic) as an axiom.
> I agree that the ide-generic could be more verbose and print more
> information in case of skipping the probe (patches are welcomed).
>
I think this is a good idea, but more important IMHO is to document
properly in the code all this discussion. If I have some spare time
(not sure about this), I'll try to submit patches (better to have two)
to address both "issues".
> > > > > Moreover this patch introduces a regression. In the situation
> > > > > when there are no PCI IDE devices and the probing should be done
> > > > > automatically (for the first two legacy IDE ports) it will be no
> > > > > longer done.
> > > > >
> >
> > Again, please elaborate more on this.
> >
> > If there are no PCI IDE devices, well, there are no PCI IDE devices.
> > Nothing is supposed to be found. Anyway, it seems they will be searched
> > for, in ide_generic_check_pci_legacy_iobases(). Or I missed something
> > important.
>
> In case there are no PCI IDE devices we most likely are running on
> pre-PCI system and should be probing for legacy ISA IDE ports (please
> note they are are using the same I/O resources),
>
Ok, that is, ide-generic is tailored to pure ISA devices, and _can_ work
in some PCI devices, although not guaranteed, on user request
(probe_mask!=0).
> > > > > Now back to the using correct PCI IDE host drivers - Luiz what
> > > > > are the systems that you need this patch on? Could you please
> > > > > get 'lspci -nn' command output from them?
> > > >
> > > > The original code before the patch in question probed the interfaces
> > > > unconditionally, probe_mask was a static int set to "0x03".
> > > >
> > > > Commit 20df429dd6671804999493baf2952f82582869fa changed the default
> > > > behavior, as well as adding a new module parameter whose behavior
> > > > makes no sense at all. Inverted bit logic? Give me a break.
> > > >
> > > > Sorry, no, the fix is correct and I'm pushing it to Linus.
> > >
> > > The "fix" is not correct and is not needed. 99% of users of ide-generic
> > > used it by mistake and should have used the designated host drivers for
> > > their hardware or PCI IDE generic host driver (not ide-generic one).
> >
> > My system didn't work without this patch. I agree I could use the
> > designated host driver, but it seems a little strong to say one _should_
> > use it.
>
> Sorry but one should really use the designated drivers and we don't
> want to see any bugreports from using ide-generic on hardware that
> have designated drivers.
>
As said above, this is a kind of axiom, and I understand this is a
conscious decision made somewhere in the past.
Avoiding extra bug reports is a smart decision, anyway.
> > > Alan Cox did the work on fixing this for his pata_legacy libata host
> > > driver and later Borislav back-ported needed changes to ide-generic host
> > > driver in commit 20df429dd6671804999493baf2952f82582869fa (in *2008*).
> > >
> > > Also the "fix" is not a revert but a new patch which introduces a real
> > > regression described by me in the previous mail.
> > >
> >
> > As said above, it is a regression considering the versions from 2006. I
> > agree that for someone who used the drivers from 2009 and after, it
> > _may_ be a regression if she uses to bring all up with probe_mask=0.
>
> The regression I was talking about is in the proposed patch.
>
> By simply reversing the logic ISA IDE ports will no longer be probed
> automatically on non-PCI systems.
>
Understood. It makes sense.
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: updated sata node on ls1046a dts
From: Shawn Guo @ 2016-12-30 2:36 UTC (permalink / raw)
To: yuantian.tang-3arQi8VN3Tc
Cc: tj-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, linux-ide-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1479369560-9188-2-git-send-email-yuantian.tang-3arQi8VN3Tc@public.gmane.org>
On Thu, Nov 17, 2016 at 03:59:20PM +0800, yuantian.tang-3arQi8VN3Tc@public.gmane.org wrote:
> From: Tang Yuantian <Yuantian.Tang-3arQi8VN3Tc@public.gmane.org>
>
> On ls1046a soc, sata ecc should be disabled. So added sata ecc
disabled or enabled?
Shawn
> register address so that driver can get this information.
>
> Signed-off-by: Tang Yuantian <yuantian.tang-3arQi8VN3Tc@public.gmane.org>
> ---
> arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> index 38806ca..88aaaf1 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> @@ -507,7 +507,9 @@
>
> sata: sata@3200000 {
> compatible = "fsl,ls1046a-ahci";
> - reg = <0x0 0x3200000 0x0 0x10000>;
> + reg = <0x0 0x3200000 0x0 0x10000>,
> + <0x0 0x20140520 0x0 0x4>;
> + reg-names = "ahci", "sata-ecc";
> interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&clockgen 4 1>;
> };
> --
> 2.1.0.27.g96db324
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] ide: palm_bk3710: add __initdata to palm_bk3710_port_info
From: Bhumika Goyal @ 2016-12-30 9:20 UTC (permalink / raw)
To: julia.lawall, davem, linux-ide, linux-kernel; +Cc: Bhumika Goyal
The object palm_bk3710_port_info of type ide_port_info is never
referenced anywhere after initialization by palm_bk3710_probe. It is
also passed as a parameter to ide_host_add which is called from the init
function but this call doesn't store the object reference anywhere, and
it only dereferences the values of the fields. Therefore add __initdata
to its declaration.
Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
Compiled, but not tested
drivers/ide/palm_bk3710.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ide/palm_bk3710.c b/drivers/ide/palm_bk3710.c
index 46427ea..157f2d1 100644
--- a/drivers/ide/palm_bk3710.c
+++ b/drivers/ide/palm_bk3710.c
@@ -300,7 +300,7 @@ static int palm_bk3710_init_dma(ide_hwif_t *hwif, const struct ide_port_info *d)
.cable_detect = palm_bk3710_cable_detect,
};
-static struct ide_port_info palm_bk3710_port_info = {
+static struct ide_port_info palm_bk3710_port_info __initdata = {
.init_dma = palm_bk3710_init_dma,
.port_ops = &palm_bk3710_ports_ops,
.dma_ops = &sff_dma_ops,
--
1.9.1
^ permalink raw reply related
* [PATCH 0/3] ata: add m68k/Atari Falcon PATA support
From: Bartlomiej Zolnierkiewicz @ 2016-12-30 14:01 UTC (permalink / raw)
To: Tejun Heo
Cc: Geert Uytterhoeven, Michael Schmitz, linux-ide, linux-m68k,
linux-kernel, b.zolnierkie
In-Reply-To: <CGME20161230140139epcas5p160eda5a6a77be084e21f12002c85cc2a@epcas5p1.samsung.com>
Hi,
This patchset adds m68k/Atari Falcon PATA support to libata.
The major difference in the new libata's pata_falcon host
driver when compared to legacy IDE's falconide host driver is
that we are using polled PIO mode and thus avoiding the need
for STDMA locking magic altogether.
Tested under ARAnyM emulator.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Bartlomiej Zolnierkiewicz (3):
ata: allow subsystem to be used on m68k arch
ata: pass queued command to ->sff_data_xfer method
ata: add Atari Falcon PATA controller driver
drivers/ata/Kconfig | 11 ++-
drivers/ata/Makefile | 1 +
drivers/ata/libata-sff.c | 29 +++----
drivers/ata/pata_at91.c | 6 +-
drivers/ata/pata_bf54x.c | 7 +-
drivers/ata/pata_ep93xx.c | 4 +-
drivers/ata/pata_falcon.c | 184 ++++++++++++++++++++++++++++++++++++++++++
drivers/ata/pata_ixp4xx_cf.c | 4 +-
drivers/ata/pata_legacy.c | 15 ++--
drivers/ata/pata_octeon_cf.c | 12 +--
drivers/ata/pata_pcmcia.c | 6 +-
drivers/ata/pata_samsung_cf.c | 4 +-
drivers/ata/sata_rcar.c | 4 +-
include/linux/libata.h | 8 +-
14 files changed, 247 insertions(+), 48 deletions(-)
create mode 100644 drivers/ata/pata_falcon.c
--
1.9.1
^ permalink raw reply
* [PATCH 1/3] ata: allow subsystem to be used on m68k arch
From: Bartlomiej Zolnierkiewicz @ 2016-12-30 14:01 UTC (permalink / raw)
To: Tejun Heo
Cc: Geert Uytterhoeven, Michael Schmitz, linux-ide, linux-m68k,
linux-kernel, b.zolnierkie
In-Reply-To: <1483106478-1382-1-git-send-email-b.zolnierkie@samsung.com>
When libata was merged m68k lacked IOMAP support. This has not been
true for a long time now so allow subsystem to be used on m68k.
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
drivers/ata/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 2c8be74..da19fc9 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -14,7 +14,7 @@ menuconfig ATA
tristate "Serial ATA and Parallel ATA drivers (libata)"
depends on HAS_IOMEM
depends on BLOCK
- depends on !(M32R || M68K || S390) || BROKEN
+ depends on !(M32R || S390) || BROKEN
select SCSI
select GLOB
---help---
--
1.9.1
^ permalink raw reply related
* [PATCH 3/3] ata: add Atari Falcon PATA controller driver
From: Bartlomiej Zolnierkiewicz @ 2016-12-30 14:01 UTC (permalink / raw)
To: Tejun Heo
Cc: Geert Uytterhoeven, Michael Schmitz, linux-ide, linux-m68k,
linux-kernel, b.zolnierkie
In-Reply-To: <1483106478-1382-1-git-send-email-b.zolnierkie@samsung.com>
Add Atari Falcon PATA controller driver. The major difference
when compared to legacy IDE's falconide host driver is that we
are using polled PIO mode and thus avoiding the need for STDMA
locking magic altogether.
Tested under ARAnyM emulator.
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
drivers/ata/Kconfig | 9 +++
drivers/ata/Makefile | 1 +
drivers/ata/pata_falcon.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 194 insertions(+)
create mode 100644 drivers/ata/pata_falcon.c
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index da19fc9..cbd1019 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -895,6 +895,15 @@ config PATA_CMD640_PCI
If unsure, say N.
+config PATA_FALCON
+ tristate "Atari Falcon PATA support"
+ depends on M68K && ATARI
+ help
+ This option enables support for the on-board IDE
+ interface on the Atari Falcon.
+
+ If unsure, say N.
+
config PATA_ISAPNP
tristate "ISA Plug and Play PATA support"
depends on ISAPNP
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index a46e6b7..89a0a19 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -93,6 +93,7 @@ obj-$(CONFIG_PATA_WINBOND) += pata_sl82c105.o
obj-$(CONFIG_PATA_AT32) += pata_at32.o
obj-$(CONFIG_PATA_AT91) += pata_at91.o
obj-$(CONFIG_PATA_CMD640_PCI) += pata_cmd640.o
+obj-$(CONFIG_PATA_FALCON) += pata_falcon.o
obj-$(CONFIG_PATA_ISAPNP) += pata_isapnp.o
obj-$(CONFIG_PATA_IXP4XX_CF) += pata_ixp4xx_cf.o
obj-$(CONFIG_PATA_MPIIX) += pata_mpiix.o
diff --git a/drivers/ata/pata_falcon.c b/drivers/ata/pata_falcon.c
new file mode 100644
index 0000000..7826408
--- /dev/null
+++ b/drivers/ata/pata_falcon.c
@@ -0,0 +1,184 @@
+/*
+ * Atari Falcon PATA controller driver
+ *
+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * Based on falconide.c:
+ *
+ * Created 12 Jul 1997 by Geert Uytterhoeven
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/blkdev.h>
+#include <linux/delay.h>
+#include <scsi/scsi_host.h>
+#include <scsi/scsi_cmnd.h>
+#include <linux/ata.h>
+#include <linux/libata.h>
+#include <linux/mm.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+
+#include <asm/setup.h>
+#include <asm/atarihw.h>
+#include <asm/atariints.h>
+#include <asm/atari_stdma.h>
+#include <asm/ide.h>
+
+#define DRV_NAME "pata_falcon"
+#define DRV_VERSION "0.1.0"
+
+#define ATA_HD_BASE 0xfff00000
+#define ATA_HD_CONTROL 0x39
+
+static struct scsi_host_template pata_falcon_sht = {
+ ATA_PIO_SHT(DRV_NAME),
+};
+
+static unsigned int pata_falcon_data_xfer(struct ata_queued_cmd *qc,
+ unsigned char *buf,
+ unsigned int buflen, int rw)
+{
+ struct ata_device *dev = qc->dev;
+ struct ata_port *ap = dev->link->ap;
+ void __iomem *data_addr = ap->ioaddr.data_addr;
+ unsigned int words = buflen >> 1;
+ struct scsi_cmnd *cmd = qc->scsicmd;
+ bool swap = 1;
+
+ if (dev->class == ATA_DEV_ATA && cmd && cmd->request &&
+ cmd->request->cmd_type == REQ_TYPE_FS)
+ swap = 0;
+
+ /* Transfer multiple of 2 bytes */
+ if (rw == READ) {
+ if (swap)
+ raw_insw_swapw((u16 *)data_addr, (u16 *)buf, words);
+ else
+ raw_insw((u16 *)data_addr, (u16 *)buf, words);
+ } else {
+ if (swap)
+ raw_outsw_swapw((u16 *)data_addr, (u16 *)buf, words);
+ else
+ raw_outsw((u16 *)data_addr, (u16 *)buf, words);
+ }
+
+ /* Transfer trailing byte, if any. */
+ if (unlikely(buflen & 0x01)) {
+ unsigned char pad[2] = { };
+
+ /* Point buf to the tail of buffer */
+ buf += buflen - 1;
+
+ if (rw == READ) {
+ if (swap)
+ raw_insw_swapw((u16 *)data_addr, (u16 *)pad, 1);
+ else
+ raw_insw((u16 *)data_addr, (u16 *)pad, 1);
+ *buf = pad[0];
+ } else {
+ pad[0] = *buf;
+ if (swap)
+ raw_outsw_swapw((u16 *)data_addr, (u16 *)pad, 1);
+ else
+ raw_outsw((u16 *)data_addr, (u16 *)pad, 1);
+ }
+ words++;
+ }
+
+ return words << 1;
+}
+
+/*
+ * Provide our own set_mode() as we don't want to change anything that has
+ * already been configured..
+ */
+static int pata_falcon_set_mode(struct ata_link *link,
+ struct ata_device **unused)
+{
+ struct ata_device *dev;
+
+ ata_for_each_dev(dev, link, ENABLED) {
+ /* We don't really care */
+ dev->pio_mode = dev->xfer_mode = XFER_PIO_0;
+ dev->xfer_shift = ATA_SHIFT_PIO;
+ dev->flags |= ATA_DFLAG_PIO;
+ ata_dev_info(dev, "configured for PIO\n");
+ }
+ return 0;
+}
+
+static struct ata_port_operations pata_falcon_ops = {
+ .inherits = &ata_sff_port_ops,
+ .sff_data_xfer = pata_falcon_data_xfer,
+ .cable_detect = ata_cable_unknown,
+ .set_mode = pata_falcon_set_mode,
+};
+
+static int pata_falcon_init_one(void)
+{
+ struct ata_host *host;
+ struct ata_port *ap;
+ struct platform_device *pdev;
+ void __iomem *base;
+
+ if (!MACH_IS_ATARI || !ATARIHW_PRESENT(IDE))
+ return -ENODEV;
+
+ pr_info(DRV_NAME ": Atari Falcon PATA controller\n");
+
+ pdev = platform_device_register_simple(DRV_NAME, 0, NULL, 0);
+ if (IS_ERR(pdev))
+ return PTR_ERR(pdev);
+
+ if (!devm_request_mem_region(&pdev->dev, ATA_HD_BASE, 0x40, DRV_NAME)) {
+ pr_err(DRV_NAME ": resources busy\n");
+ return -EBUSY;
+ }
+
+ /* allocate host */
+ host = ata_host_alloc(&pdev->dev, 1);
+ if (!host)
+ return -ENOMEM;
+ ap = host->ports[0];
+
+ ap->ops = &pata_falcon_ops;
+ ap->pio_mask = ATA_PIO4;
+ ap->flags |= ATA_FLAG_SLAVE_POSS | ATA_FLAG_NO_IORDY;
+ ap->flags |= ATA_FLAG_PIO_POLLING;
+
+ base = (void __iomem *)ATA_HD_BASE;
+ ap->ioaddr.data_addr = base;
+ ap->ioaddr.error_addr = base + 1 + 1 * 4;
+ ap->ioaddr.feature_addr = base + 1 + 1 * 4;
+ ap->ioaddr.nsect_addr = base + 1 + 2 * 4;
+ ap->ioaddr.lbal_addr = base + 1 + 3 * 4;
+ ap->ioaddr.lbam_addr = base + 1 + 4 * 4;
+ ap->ioaddr.lbah_addr = base + 1 + 5 * 4;
+ ap->ioaddr.device_addr = base + 1 + 6 * 4;
+ ap->ioaddr.status_addr = base + 1 + 7 * 4;
+ ap->ioaddr.command_addr = base + 1 + 7 * 4;
+
+ ap->ioaddr.altstatus_addr = base + ATA_HD_CONTROL;
+ ap->ioaddr.ctl_addr = base + ATA_HD_CONTROL;
+
+ ata_port_desc(ap, "cmd 0x%lx ctl 0x%lx", (unsigned long)base,
+ (unsigned long)base + ATA_HD_CONTROL);
+
+ /* activate */
+ return ata_host_activate(host, 0, NULL, 0, &pata_falcon_sht);
+}
+
+module_init(pata_falcon_init_one);
+
+MODULE_AUTHOR("Bartlomiej Zolnierkiewicz");
+MODULE_DESCRIPTION("low-level driver for Atari Falcon PATA");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(DRV_VERSION);
--
1.9.1
^ permalink raw reply related
* [PATCH 2/3] ata: pass queued command to ->sff_data_xfer method
From: Bartlomiej Zolnierkiewicz @ 2016-12-30 14:01 UTC (permalink / raw)
To: Tejun Heo
Cc: Geert Uytterhoeven, Michael Schmitz, linux-ide, linux-m68k,
linux-kernel, b.zolnierkie
In-Reply-To: <1483106478-1382-1-git-send-email-b.zolnierkie@samsung.com>
For Atari Falcon PATA support we need to check the current command
in its ->sff_data_xfer method. Update core code and all users
accordingly.
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
drivers/ata/libata-sff.c | 29 +++++++++++++++--------------
drivers/ata/pata_at91.c | 6 +++---
drivers/ata/pata_bf54x.c | 7 ++++---
drivers/ata/pata_ep93xx.c | 4 ++--
drivers/ata/pata_ixp4xx_cf.c | 4 ++--
drivers/ata/pata_legacy.c | 15 +++++++++------
drivers/ata/pata_octeon_cf.c | 12 ++++++------
drivers/ata/pata_pcmcia.c | 6 +++---
drivers/ata/pata_samsung_cf.c | 4 ++--
drivers/ata/sata_rcar.c | 4 ++--
include/linux/libata.h | 8 ++++----
11 files changed, 52 insertions(+), 47 deletions(-)
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 051b615..4441b5c 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -542,7 +542,7 @@ static inline void ata_tf_to_host(struct ata_port *ap,
/**
* ata_sff_data_xfer - Transfer data by PIO
- * @dev: device to target
+ * @qc: queued command
* @buf: data buffer
* @buflen: buffer length
* @rw: read/write
@@ -555,10 +555,10 @@ static inline void ata_tf_to_host(struct ata_port *ap,
* RETURNS:
* Bytes consumed.
*/
-unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf,
+unsigned int ata_sff_data_xfer(struct ata_queued_cmd *qc, unsigned char *buf,
unsigned int buflen, int rw)
{
- struct ata_port *ap = dev->link->ap;
+ struct ata_port *ap = qc->dev->link->ap;
void __iomem *data_addr = ap->ioaddr.data_addr;
unsigned int words = buflen >> 1;
@@ -595,7 +595,7 @@ unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf,
/**
* ata_sff_data_xfer32 - Transfer data by PIO
- * @dev: device to target
+ * @qc: queued command
* @buf: data buffer
* @buflen: buffer length
* @rw: read/write
@@ -610,16 +610,17 @@ unsigned int ata_sff_data_xfer(struct ata_device *dev, unsigned char *buf,
* Bytes consumed.
*/
-unsigned int ata_sff_data_xfer32(struct ata_device *dev, unsigned char *buf,
+unsigned int ata_sff_data_xfer32(struct ata_queued_cmd *qc, unsigned char *buf,
unsigned int buflen, int rw)
{
+ struct ata_device *dev = qc->dev;
struct ata_port *ap = dev->link->ap;
void __iomem *data_addr = ap->ioaddr.data_addr;
unsigned int words = buflen >> 2;
int slop = buflen & 3;
if (!(ap->pflags & ATA_PFLAG_PIO32))
- return ata_sff_data_xfer(dev, buf, buflen, rw);
+ return ata_sff_data_xfer(qc, buf, buflen, rw);
/* Transfer multiple of 4 bytes */
if (rw == READ)
@@ -658,7 +659,7 @@ unsigned int ata_sff_data_xfer32(struct ata_device *dev, unsigned char *buf,
/**
* ata_sff_data_xfer_noirq - Transfer data by PIO
- * @dev: device to target
+ * @qc: queued command
* @buf: data buffer
* @buflen: buffer length
* @rw: read/write
@@ -672,14 +673,14 @@ unsigned int ata_sff_data_xfer32(struct ata_device *dev, unsigned char *buf,
* RETURNS:
* Bytes consumed.
*/
-unsigned int ata_sff_data_xfer_noirq(struct ata_device *dev, unsigned char *buf,
+unsigned int ata_sff_data_xfer_noirq(struct ata_queued_cmd *qc, unsigned char *buf,
unsigned int buflen, int rw)
{
unsigned long flags;
unsigned int consumed;
local_irq_save(flags);
- consumed = ata_sff_data_xfer32(dev, buf, buflen, rw);
+ consumed = ata_sff_data_xfer32(qc, buf, buflen, rw);
local_irq_restore(flags);
return consumed;
@@ -723,14 +724,14 @@ static void ata_pio_sector(struct ata_queued_cmd *qc)
buf = kmap_atomic(page);
/* do the actual data transfer */
- ap->ops->sff_data_xfer(qc->dev, buf + offset, qc->sect_size,
+ ap->ops->sff_data_xfer(qc, buf + offset, qc->sect_size,
do_write);
kunmap_atomic(buf);
local_irq_restore(flags);
} else {
buf = page_address(page);
- ap->ops->sff_data_xfer(qc->dev, buf + offset, qc->sect_size,
+ ap->ops->sff_data_xfer(qc, buf + offset, qc->sect_size,
do_write);
}
@@ -791,7 +792,7 @@ static void atapi_send_cdb(struct ata_port *ap, struct ata_queued_cmd *qc)
DPRINTK("send cdb\n");
WARN_ON_ONCE(qc->dev->cdb_len < 12);
- ap->ops->sff_data_xfer(qc->dev, qc->cdb, qc->dev->cdb_len, 1);
+ ap->ops->sff_data_xfer(qc, qc->cdb, qc->dev->cdb_len, 1);
ata_sff_sync(ap);
/* FIXME: If the CDB is for DMA do we need to do the transition delay
or is bmdma_start guaranteed to do it ? */
@@ -868,14 +869,14 @@ static int __atapi_pio_bytes(struct ata_queued_cmd *qc, unsigned int bytes)
buf = kmap_atomic(page);
/* do the actual data transfer */
- consumed = ap->ops->sff_data_xfer(dev, buf + offset,
+ consumed = ap->ops->sff_data_xfer(qc, buf + offset,
count, rw);
kunmap_atomic(buf);
local_irq_restore(flags);
} else {
buf = page_address(page);
- consumed = ap->ops->sff_data_xfer(dev, buf + offset,
+ consumed = ap->ops->sff_data_xfer(qc, buf + offset,
count, rw);
}
diff --git a/drivers/ata/pata_at91.c b/drivers/ata/pata_at91.c
index 1611e0e..fd5b34f 100644
--- a/drivers/ata/pata_at91.c
+++ b/drivers/ata/pata_at91.c
@@ -286,10 +286,10 @@ static void pata_at91_set_piomode(struct ata_port *ap, struct ata_device *adev)
set_smc_timing(ap->dev, adev, info, &timing);
}
-static unsigned int pata_at91_data_xfer_noirq(struct ata_device *dev,
+static unsigned int pata_at91_data_xfer_noirq(struct ata_queued_cmd *qc,
unsigned char *buf, unsigned int buflen, int rw)
{
- struct at91_ide_info *info = dev->link->ap->host->private_data;
+ struct at91_ide_info *info = qc->dev->link->ap->host->private_data;
unsigned int consumed;
unsigned int mode;
unsigned long flags;
@@ -301,7 +301,7 @@ static unsigned int pata_at91_data_xfer_noirq(struct ata_device *dev,
regmap_fields_write(fields.mode, info->cs, (mode & ~AT91_SMC_DBW) |
AT91_SMC_DBW_16);
- consumed = ata_sff_data_xfer(dev, buf, buflen, rw);
+ consumed = ata_sff_data_xfer(qc, buf, buflen, rw);
/* restore 8bit mode after data is written */
regmap_fields_write(fields.mode, info->cs, (mode & ~AT91_SMC_DBW) |
diff --git a/drivers/ata/pata_bf54x.c b/drivers/ata/pata_bf54x.c
index ec748d3..9c5780a 100644
--- a/drivers/ata/pata_bf54x.c
+++ b/drivers/ata/pata_bf54x.c
@@ -1143,7 +1143,7 @@ static unsigned char bfin_bmdma_status(struct ata_port *ap)
/**
* bfin_data_xfer - Transfer data by PIO
- * @adev: device for this I/O
+ * @qc: queued command
* @buf: data buffer
* @buflen: buffer length
* @write_data: read/write
@@ -1151,10 +1151,11 @@ static unsigned char bfin_bmdma_status(struct ata_port *ap)
* Note: Original code is ata_sff_data_xfer().
*/
-static unsigned int bfin_data_xfer(struct ata_device *dev, unsigned char *buf,
+static unsigned int bfin_data_xfer(struct ata_queued_cmd *qc,
+ unsigned char *buf,
unsigned int buflen, int rw)
{
- struct ata_port *ap = dev->link->ap;
+ struct ata_port *ap = qc->dev->link->ap;
void __iomem *base = (void __iomem *)ap->ioaddr.ctl_addr;
unsigned int words = buflen >> 1;
unsigned short *buf16 = (u16 *)buf;
diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
index bd6b089..bf1b910 100644
--- a/drivers/ata/pata_ep93xx.c
+++ b/drivers/ata/pata_ep93xx.c
@@ -474,11 +474,11 @@ static void ep93xx_pata_set_devctl(struct ata_port *ap, u8 ctl)
}
/* Note: original code is ata_sff_data_xfer */
-static unsigned int ep93xx_pata_data_xfer(struct ata_device *adev,
+static unsigned int ep93xx_pata_data_xfer(struct ata_queued_cmd *qc,
unsigned char *buf,
unsigned int buflen, int rw)
{
- struct ata_port *ap = adev->link->ap;
+ struct ata_port *ap = qc->dev->link->ap;
struct ep93xx_pata_data *drv_data = ap->host->private_data;
u16 *data = (u16 *)buf;
unsigned int words = buflen >> 1;
diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
index abda441..0b0d930 100644
--- a/drivers/ata/pata_ixp4xx_cf.c
+++ b/drivers/ata/pata_ixp4xx_cf.c
@@ -40,13 +40,13 @@ static int ixp4xx_set_mode(struct ata_link *link, struct ata_device **error)
return 0;
}
-static unsigned int ixp4xx_mmio_data_xfer(struct ata_device *dev,
+static unsigned int ixp4xx_mmio_data_xfer(struct ata_queued_cmd *qc,
unsigned char *buf, unsigned int buflen, int rw)
{
unsigned int i;
unsigned int words = buflen >> 1;
u16 *buf16 = (u16 *) buf;
- struct ata_port *ap = dev->link->ap;
+ struct ata_port *ap = qc->dev->link->ap;
void __iomem *mmio = ap->ioaddr.data_addr;
struct ixp4xx_pata_data *data = dev_get_platdata(ap->host->dev);
diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
index bce2a8c..53828b6c 100644
--- a/drivers/ata/pata_legacy.c
+++ b/drivers/ata/pata_legacy.c
@@ -303,11 +303,12 @@ static void pdc20230_set_piomode(struct ata_port *ap, struct ata_device *adev)
}
-static unsigned int pdc_data_xfer_vlb(struct ata_device *dev,
+static unsigned int pdc_data_xfer_vlb(struct ata_queued_cmd *qc,
unsigned char *buf, unsigned int buflen, int rw)
{
- int slop = buflen & 3;
+ struct ata_device *dev = qc->dev;
struct ata_port *ap = dev->link->ap;
+ int slop = buflen & 3;
/* 32bit I/O capable *and* we need to write a whole number of dwords */
if (ata_id_has_dword_io(dev->id) && (slop == 0 || slop == 3)
@@ -340,7 +341,7 @@ static unsigned int pdc_data_xfer_vlb(struct ata_device *dev,
}
local_irq_restore(flags);
} else
- buflen = ata_sff_data_xfer_noirq(dev, buf, buflen, rw);
+ buflen = ata_sff_data_xfer_noirq(qc, buf, buflen, rw);
return buflen;
}
@@ -702,9 +703,11 @@ static unsigned int qdi_qc_issue(struct ata_queued_cmd *qc)
return ata_sff_qc_issue(qc);
}
-static unsigned int vlb32_data_xfer(struct ata_device *adev, unsigned char *buf,
- unsigned int buflen, int rw)
+static unsigned int vlb32_data_xfer(struct ata_queued_cmd *qc,
+ unsigned char *buf,
+ unsigned int buflen, int rw)
{
+ struct ata_device *adev = qc->dev;
struct ata_port *ap = adev->link->ap;
int slop = buflen & 3;
@@ -727,7 +730,7 @@ static unsigned int vlb32_data_xfer(struct ata_device *adev, unsigned char *buf,
}
return (buflen + 3) & ~3;
} else
- return ata_sff_data_xfer(adev, buf, buflen, rw);
+ return ata_sff_data_xfer(qc, buf, buflen, rw);
}
static int qdi_port(struct platform_device *dev,
diff --git a/drivers/ata/pata_octeon_cf.c b/drivers/ata/pata_octeon_cf.c
index 475a006..e94e7ca 100644
--- a/drivers/ata/pata_octeon_cf.c
+++ b/drivers/ata/pata_octeon_cf.c
@@ -293,17 +293,17 @@ static void octeon_cf_set_dmamode(struct ata_port *ap, struct ata_device *dev)
/**
* Handle an 8 bit I/O request.
*
- * @dev: Device to access
+ * @qc: Queued command
* @buffer: Data buffer
* @buflen: Length of the buffer.
* @rw: True to write.
*/
-static unsigned int octeon_cf_data_xfer8(struct ata_device *dev,
+static unsigned int octeon_cf_data_xfer8(struct ata_queued_cmd *qc,
unsigned char *buffer,
unsigned int buflen,
int rw)
{
- struct ata_port *ap = dev->link->ap;
+ struct ata_port *ap = qc->dev->link->ap;
void __iomem *data_addr = ap->ioaddr.data_addr;
unsigned long words;
int count;
@@ -332,17 +332,17 @@ static unsigned int octeon_cf_data_xfer8(struct ata_device *dev,
/**
* Handle a 16 bit I/O request.
*
- * @dev: Device to access
+ * @qc: Queued command
* @buffer: Data buffer
* @buflen: Length of the buffer.
* @rw: True to write.
*/
-static unsigned int octeon_cf_data_xfer16(struct ata_device *dev,
+static unsigned int octeon_cf_data_xfer16(struct ata_queued_cmd *qc,
unsigned char *buffer,
unsigned int buflen,
int rw)
{
- struct ata_port *ap = dev->link->ap;
+ struct ata_port *ap = qc->dev->link->ap;
void __iomem *data_addr = ap->ioaddr.data_addr;
unsigned long words;
int count;
diff --git a/drivers/ata/pata_pcmcia.c b/drivers/ata/pata_pcmcia.c
index bcc4b96..a541eac 100644
--- a/drivers/ata/pata_pcmcia.c
+++ b/drivers/ata/pata_pcmcia.c
@@ -90,7 +90,7 @@ static int pcmcia_set_mode_8bit(struct ata_link *link,
/**
* ata_data_xfer_8bit - Transfer data by 8bit PIO
- * @dev: device to target
+ * @qc: queued command
* @buf: data buffer
* @buflen: buffer length
* @rw: read/write
@@ -101,10 +101,10 @@ static int pcmcia_set_mode_8bit(struct ata_link *link,
* Inherited from caller.
*/
-static unsigned int ata_data_xfer_8bit(struct ata_device *dev,
+static unsigned int ata_data_xfer_8bit(struct ata_queued_cmd *qc,
unsigned char *buf, unsigned int buflen, int rw)
{
- struct ata_port *ap = dev->link->ap;
+ struct ata_port *ap = qc->dev->link->ap;
if (rw == READ)
ioread8_rep(ap->ioaddr.data_addr, buf, buflen);
diff --git a/drivers/ata/pata_samsung_cf.c b/drivers/ata/pata_samsung_cf.c
index f6facd6..431c7de 100644
--- a/drivers/ata/pata_samsung_cf.c
+++ b/drivers/ata/pata_samsung_cf.c
@@ -263,10 +263,10 @@ static u8 pata_s3c_check_altstatus(struct ata_port *ap)
/*
* pata_s3c_data_xfer - Transfer data by PIO
*/
-static unsigned int pata_s3c_data_xfer(struct ata_device *dev,
+static unsigned int pata_s3c_data_xfer(struct ata_queued_cmd *qc,
unsigned char *buf, unsigned int buflen, int rw)
{
- struct ata_port *ap = dev->link->ap;
+ struct ata_port *ap = qc->dev->link->ap;
struct s3c_ide_info *info = ap->host->private_data;
void __iomem *data_addr = ap->ioaddr.data_addr;
unsigned int words = buflen >> 1, i;
diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index f72d601..5d38245 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -447,11 +447,11 @@ static void sata_rcar_exec_command(struct ata_port *ap,
ata_sff_pause(ap);
}
-static unsigned int sata_rcar_data_xfer(struct ata_device *dev,
+static unsigned int sata_rcar_data_xfer(struct ata_queued_cmd *qc,
unsigned char *buf,
unsigned int buflen, int rw)
{
- struct ata_port *ap = dev->link->ap;
+ struct ata_port *ap = qc->dev->link->ap;
void __iomem *data_addr = ap->ioaddr.data_addr;
unsigned int words = buflen >> 1;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index c170be5..0e8a800 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -968,7 +968,7 @@ struct ata_port_operations {
void (*sff_tf_read)(struct ata_port *ap, struct ata_taskfile *tf);
void (*sff_exec_command)(struct ata_port *ap,
const struct ata_taskfile *tf);
- unsigned int (*sff_data_xfer)(struct ata_device *dev,
+ unsigned int (*sff_data_xfer)(struct ata_queued_cmd *qc,
unsigned char *buf, unsigned int buflen, int rw);
void (*sff_irq_on)(struct ata_port *);
bool (*sff_irq_check)(struct ata_port *);
@@ -1823,11 +1823,11 @@ extern int ata_sff_busy_sleep(struct ata_port *ap,
extern void ata_sff_tf_read(struct ata_port *ap, struct ata_taskfile *tf);
extern void ata_sff_exec_command(struct ata_port *ap,
const struct ata_taskfile *tf);
-extern unsigned int ata_sff_data_xfer(struct ata_device *dev,
+extern unsigned int ata_sff_data_xfer(struct ata_queued_cmd *qc,
unsigned char *buf, unsigned int buflen, int rw);
-extern unsigned int ata_sff_data_xfer32(struct ata_device *dev,
+extern unsigned int ata_sff_data_xfer32(struct ata_queued_cmd *qc,
unsigned char *buf, unsigned int buflen, int rw);
-extern unsigned int ata_sff_data_xfer_noirq(struct ata_device *dev,
+extern unsigned int ata_sff_data_xfer_noirq(struct ata_queued_cmd *qc,
unsigned char *buf, unsigned int buflen, int rw);
extern void ata_sff_irq_on(struct ata_port *ap);
extern void ata_sff_irq_clear(struct ata_port *ap);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 1/3] ata: allow subsystem to be used on m68k arch
From: Christoph Hellwig @ 2016-12-30 14:12 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Tejun Heo, Geert Uytterhoeven, Michael Schmitz, linux-ide,
linux-m68k, linux-kernel
In-Reply-To: <1483106478-1382-2-git-send-email-b.zolnierkie@samsung.com>
On Fri, Dec 30, 2016 at 03:01:16PM +0100, Bartlomiej Zolnierkiewicz wrote:
> When libata was merged m68k lacked IOMAP support. This has not been
> true for a long time now so allow subsystem to be used on m68k.
Is that dependency needed at all anymore? What exact functionality
is missing on m32r and s390?
^ permalink raw reply
* Re: [PATCH] Fix interface autodetection in legacy IDE driver (trial #2)
From: Bartlomiej Zolnierkiewicz @ 2016-12-30 16:05 UTC (permalink / raw)
To: Luiz Carlos Ramos; +Cc: David Miller, linux-ide, petkovbb
In-Reply-To: <20161230005232.GA9069@giustizia>
Hi,
[ Your mails don't make it into linux-ide mailing list for some reason
(the reason why I have not seen your patch before Dave applied it). ]
On Thursday, December 29, 2016 10:52:33 PM Luiz Carlos Ramos wrote:
> Hi,
>
> I think I could catch the whole picture after your explanation. I'll try
> to summarize all the points discussed below in the body.
>
> My suggestion now is to drop the current (proposed) patch and substitute
> it for a pure "documentational" patch, as it seems for me that a
> programmer new to that subsystem - as the case of myself - tends to find
> ide-generic.c buggy, if analyzed out of the whole context. Some more
> inline documentation may help all of us IMHO.
I agree. BTW ide-generic is a bad name (my fault), it should have been
ide-isa or (better) ide-legacy.
> Of course improving the printed messages - as suggested - can help as well.
>
> I'd like to hear from David (cc'ed) about this suggestion. What would you David
> suggest?
>
> Anyway, there is a "break of the contract" with the old patch which
> introduced the search for the IDE interfaces. Before, it was not necessary
> to specify probe_mask=0x03 or like to make ide-generic find something in
> I/O addresses 0x1f0 and 0x170 (I wrongly wrote 0x1e0 in the former emails;
> please apologize me for that mistake). But this contract break seems to be
> justifiable on the grounds that - by a design decision - users "are encouraged"
> to use the designated drivers as a basic axiom. I suppose (or I hope) this
> was discussed at that time and this decision was taken consciously.
Yes, it was a conscious decision.
> On Wed, Dec 28, 2016 at 12:10:16PM +0100, Bartlomiej Zolnierkiewicz wrote:
>
> > > > > >> > > This humble patch was sent one or two months before, and had no actions,
> > > > > >> > > except for a colleague reply which friendly pointed out some formatting
> > > > > >> > > problems (which were solved in a second message).
> > > > > >> > >
> > > > > >> > > It relates to an old code, the legacy IDE driver, but the bug it
> > > > > >> > > addresses is real. The code, although rarely used, is
> > > > > >> > > still there to be compiled if one chooses to do so (like me).
> > > > > >> > >
> > > > > >> > > Also, the fix has a very low risk of present collateral effects IMHO.
> > > > > >> > > It is already compiled and tested in some embedded machines.
> > > > > >> > >
> > > > > >> > > So, again IMHO, it is worth be fixed.
> > > > > >> > >
> > > > > >> > > This email is a second trial with it. I hope it can help the one or two
> > > > > >> > > guys out there which are still running the legacy IDE driver and
> > > > > >> > > haven't noticed the former email.
> > > > > >> > >
> > > > > >> > > Best regards,
> > > > > >> > >
> > > > > >> > > Signed-off-by: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
> > > > > >> >
> > > > > >> > This bug was introduced by commit
> > > > > >> > 20df429dd6671804999493baf2952f82582869fa ("ide-generic: handle probing
> > > > > >> > of legacy io-ports v5") which seems poorly tested.
> > > > > >>
> > > > > >> Please always cc: the original commit author.
> > > > > >>
> > > > > >> > Applied and queued up for -stable, th anks.
> > > > > >>
> > > > > >> For some reason I've never got the discussed patch from
> > > > > >> linux-ide ML though I now have found in the patchwork:
> > > > > >>
> > > > > >> https://patchwork.ozlabs.org/patch/680975/
> > > > > >>
> > > > > >> The patch is incorrect. If you have PCI IDE devices (like in
> > >
> > > I think all of you have checked the code, but I will try to describe
> > > what I'm seeing.
> > >
> > > The routine ide_generic_check_pci_legacy_iobases() returns *primary
> > > equal to 1 if some PCI IDE resource is found at 0x1f0, and *secondary
> > > equal to 1 if some PCI IDE resource is found at 0x1e0. The same
> > > routine doesn't change neither *primary not *secondary if nothing is
> > > found. The only caller - ide_generic_init() - initializes both to zero
> > > before the call.
> > >
> > > So, ide_generic_init() should test *primary for 1 in the case of an
> > > existing IDE primary resource, and *secondary for 1 in the case of a
> > > secondary IDE resource.
> > >
> > > Unpatched code checks both for zero in order to set the proper bits in
> > > probe_mask, and IMHO this is reversed logic.
> >
> > Unpatched code is correct.
> >
> > If PCI IDE resource is found we shouldn't do the probe automatically
> > as ide-generic is not the proper driver to run the hardware. IOW we
> > only want to do automatic probe if no PCI IDE resources are found (if
> > primary/secondary == 0). In such case we are most likely running on
> > pre-PCI system and should be probing for legacy ISA IDE ports. Please
> > note they are are using the same I/O resources as PCI IDE ones (!).
> >
>
> Understood. ide-generic should not use devices which have a PCI
> interface, unless the user tells it to do it (using probe_mask!=0).
>
> The basic assumption - as stated I think in your first reply - is that
> by design, users should use the driver more suitable for the chipset, if
> it has a PCI interface, and not ide-generic.
>
> I think this assumption should be highlighted as an axiom, or a design
> decision. Otherwise, one could argue in favor of enabling ide-generic as a first
> option for handling the IDE interface, be that PCI or IDE.
>
> Also is supposed that this decision is not subject to discussion (now).
You are correct, also IDE subsystem is now in deep maintenance mode.
> > > > > >> The patch is incorrect. If you have PCI IDE devices (like in
> > > > > >> the case described in the situation being "fixed" by the patch)
> > > > > >> you should use the correct PCI IDE host driver for proper
> > > > > >> operation and not ide-generic host driver (the latter still can
> > > > > >> be used by using kernel parameters).
> > > > > >
> > >
> > > Please elaborate more on this; I really haven't understood. It seems that
> > > you're telling about PCI host driver code, which I haven't studied. I
> > > haven't caught the whole picture.
> > >
> > > In my case, there are two IDE interfaces, which are at the common
> > > addresses 0x1f0 and 0x1e0 (PCI also tells me that), and the chipset is
> > > a CS5536. The previous kernel used in the system (2.6.16) worked with
> > > ide-generic, and the one I'm testing now (3.18.x) doesn't bring any of
> > > the IDE interfaces up.
> > >
> > > You're right in the sense I haven't compiled the CS5536 legacy host
> > > driver code; so, I can't tell about how it works.
> >
> > You should be using CS5336 PCI IDE host driver (drivers/ide/cs5536.c
> > enabled by CONFIG_BLK_DEV_CS5536 config option) for optimal performance
> > and proper operations.
> >
> > CS5536 PCI IDE host driver was added in kernel v2.6.29.
>
> This can be one reason to not have CS5536 driver used in the first
> version of the system (kernel 2.6.16).
Please note that CS5536 is supported also in amd74xx.c host driver
(for historical reasons) and this support was merged in 2005 in
commit 7fab773de16ccaeb249acdc6e956a9759c68225d (kernel v2.6.15).
Also even before that ide-pci-generic.c driver has been available
which is still more suitable than ide-generic.c one for CS5536
(it needs to be forced with "ide_pci_generic.all_generic_ide" kernel
command line parameter).
> > > Of course there are many reasons to use the correct PCI IDE host
> > > driver, but at the end, it's up to the user to choose which one he/she will
> > > use, and ideally any of them should work.
> >
> > It happens to "work" on your system because CS5536 is relatively
> > simple and non-buggy PCI IDE chipset.
> >
> > In case of other PCI IDE chipsets you may not only be missing
> > performance or some functionality - on some more quirky chipsets
> > you may be also affecting data integrity (!).
> >
> > Anyway you still can use ide-generic by using kernel parameters
> > (just add "ide_generic.probe_mask=0x03" to your kernel command line).
> >
>
> As said before, this is a contract break between versions from, let's
> say, 2006 and today's.
>
> Anyway, there's no way to avoid it, it we take the assumption above
> (designated drivers prior to ide-generic) as an axiom.
>
> > I agree that the ide-generic could be more verbose and print more
> > information in case of skipping the probe (patches are welcomed).
> >
>
> I think this is a good idea, but more important IMHO is to document
> properly in the code all this discussion. If I have some spare time
> (not sure about this), I'll try to submit patches (better to have two)
> to address both "issues".
Thank you, it would be good to have this covered.
> > > > > > Moreover this patch introduces a regression. In the situation
> > > > > > when there are no PCI IDE devices and the probing should be done
> > > > > > automatically (for the first two legacy IDE ports) it will be no
> > > > > > longer done.
> > > > > >
> > >
> > > Again, please elaborate more on this.
> > >
> > > If there are no PCI IDE devices, well, there are no PCI IDE devices.
> > > Nothing is supposed to be found. Anyway, it seems they will be searched
> > > for, in ide_generic_check_pci_legacy_iobases(). Or I missed something
> > > important.
> >
> > In case there are no PCI IDE devices we most likely are running on
> > pre-PCI system and should be probing for legacy ISA IDE ports (please
> > note they are are using the same I/O resources),
> >
>
> Ok, that is, ide-generic is tailored to pure ISA devices, and _can_ work
> in some PCI devices, although not guaranteed, on user request
> (probe_mask!=0).
Yes.
> > > > > > Now back to the using correct PCI IDE host drivers - Luiz what
> > > > > > are the systems that you need this patch on? Could you please
> > > > > > get 'lspci -nn' command output from them?
> > > > >
> > > > > The original code before the patch in question probed the interfaces
> > > > > unconditionally, probe_mask was a static int set to "0x03".
> > > > >
> > > > > Commit 20df429dd6671804999493baf2952f82582869fa changed the default
> > > > > behavior, as well as adding a new module parameter whose behavior
> > > > > makes no sense at all. Inverted bit logic? Give me a break.
> > > > >
> > > > > Sorry, no, the fix is correct and I'm pushing it to Linus.
> > > >
> > > > The "fix" is not correct and is not needed. 99% of users of ide-generic
> > > > used it by mistake and should have used the designated host drivers for
> > > > their hardware or PCI IDE generic host driver (not ide-generic one).
> > >
> > > My system didn't work without this patch. I agree I could use the
> > > designated host driver, but it seems a little strong to say one _should_
> > > use it.
> >
> > Sorry but one should really use the designated drivers and we don't
> > want to see any bugreports from using ide-generic on hardware that
> > have designated drivers.
> >
>
> As said above, this is a kind of axiom, and I understand this is a
> conscious decision made somewhere in the past.
>
> Avoiding extra bug reports is a smart decision, anyway.
>
> > > > Alan Cox did the work on fixing this for his pata_legacy libata host
> > > > driver and later Borislav back-ported needed changes to ide-generic host
> > > > driver in commit 20df429dd6671804999493baf2952f82582869fa (in *2008*).
> > > >
> > > > Also the "fix" is not a revert but a new patch which introduces a real
> > > > regression described by me in the previous mail.
> > > >
> > >
> > > As said above, it is a regression considering the versions from 2006. I
> > > agree that for someone who used the drivers from 2009 and after, it
> > > _may_ be a regression if she uses to bring all up with probe_mask=0.
> >
> > The regression I was talking about is in the proposed patch.
> >
> > By simply reversing the logic ISA IDE ports will no longer be probed
> > automatically on non-PCI systems.
> >
>
> Understood. It makes sense.
Thank you for understanding.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Re: [PATCH 1/3] ata: allow subsystem to be used on m68k arch
From: Bartlomiej Zolnierkiewicz @ 2016-12-30 17:14 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Tejun Heo, Geert Uytterhoeven, Michael Schmitz, linux-ide,
linux-m68k, linux-kernel
In-Reply-To: <20161230141249.GA4543@infradead.org>
Hi,
On Friday, December 30, 2016 06:12:49 AM Christoph Hellwig wrote:
> On Fri, Dec 30, 2016 at 03:01:16PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > When libata was merged m68k lacked IOMAP support. This has not been
> > true for a long time now so allow subsystem to be used on m68k.
>
> Is that dependency needed at all anymore? What exact functionality
> is missing on m32r and s390?
* m32r: I don't know why it was restricted but it builds fine nowadays
(I will fix it later in incremental patch)
* s390: no PATA hardware, legacy IDE subsystem is also not supported
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* Urgent Please;;
From: Dr. David White @ 2017-01-02 17:34 UTC (permalink / raw)
Dear,
With due respect to your person and much sincerity of purpose. I have a business proposal which I will like to handle with you and $14.5 Million USD is involve. But be rest assured that everything is legal and risk free as I have concluded all the arrangements and the legal papers that will back the transaction up. Kindly indicate your interest as to enable me tell you more detail of the proposal. Waiting for your urgent response.
Yours Faithfully,
Dr. David White
^ permalink raw reply
* Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support
From: Geert Uytterhoeven @ 2017-01-03 10:49 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Tejun Heo, Michael Schmitz, linux-ide@vger.kernel.org, linux-m68k,
linux-kernel@vger.kernel.org
In-Reply-To: <1483106478-1382-1-git-send-email-b.zolnierkie@samsung.com>
Hi Bartlomiej,
On Fri, Dec 30, 2016 at 3:01 PM, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
> This patchset adds m68k/Atari Falcon PATA support to libata.
Thanks for your series!
That leaves us with 4 to go ;-)
CONFIG_BLK_DEV_GAYLE
CONFIG_BLK_DEV_BUDDHA
CONFIG_BLK_DEV_MAC_IDE
CONFIG_BLK_DEV_Q40IDE
Note that using libata instead of the legacy IDE driver increases kernel size.
After enabling libata:
CONFIG_ATA=y
CONFIG_ATA_VERBOSE_ERROR=y
CONFIG_ATA_SFF=y
CONFIG_PATA_FALCON=y
an atari_defconfig kernel grew by:
add/remove: 775/0 grow/shrink: 753/41 up/down: 98999/-242 (98757)
After disabling CONFIG_IDE:
add/remove: 0/589 grow/shrink: 0/12 up/down: 0/-62835 (-62835)
So the net result is:
add/remove: 775/589 grow/shrink: 749/51 up/down: 98886/-62964 (35922)
Disabling CONFIG_ATA_VERBOSE_ERROR saved 1380 bytes, which is less than the
value advertised by Kconfig (6KB).
> The major difference in the new libata's pata_falcon host
> driver when compared to legacy IDE's falconide host driver is
> that we are using polled PIO mode and thus avoiding the need
> for STDMA locking magic altogether.
I'll let the Atari experts (Michael?) comment on that...
> Tested under ARAnyM emulator.
Works indeed fine on ARAnyM.
Unfortunately I can't test it on real hardware...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] pata_legacy: Allow disabling of legacy PATA device probes on non-PCI systems
From: One Thousand Gnomes @ 2017-01-03 17:34 UTC (permalink / raw)
To: Tejun Heo; +Cc: whiteheadm, Greg Kroah-Hartman, Sergei Shtylyov, linux-ide
In-Reply-To: <20161222192256.GE31216@htj.duckdns.org>
On Thu, 22 Dec 2016 14:22:56 -0500
Tejun Heo <tj@kernel.org> wrote:
> Hello,
>
> On Thu, Dec 22, 2016 at 01:30:23PM -0500, tedheadster wrote:
> > > I think this driver should either not be considered 'platform' or it
> > > should make a call to free_irq() on failures.
> > >
> >
> > The question is: who is responsible to free an irq from a device
> > registered using platform_device_register_simple()? Is it the driver
> > or the platform code?
>
> Sorry, I wasn't clear. There's a rudimentary garbage collector devm
> which tracks all the resources associated with a device. The previous
> patch that I sent you which created an explict resource group and
> released it uses the same mechanism. When any device gets destroyed,
> it should release all associated resources and I was wondering why
> that wasn't happening without the explicit group operations. I'll
> write up a debug patch to track down what's going on.
Almost certainly libata leaking a reference to the device so it doesn't
think it's gone away.
Alan
^ permalink raw reply
* [PATCH 0/6] ata: Missing HAS_DMA dependencies
From: Geert Uytterhoeven @ 2017-01-03 18:09 UTC (permalink / raw)
To: Tejun Heo, Bartlomiej Zolnierkiewicz
Cc: linux-ide, linux-m68k, linux-kernel, Geert Uytterhoeven
Hi all,
Bartlomiej's "[PATCH 1/3] ata: allow subsystem to be used on m68k arch"
exposed a few missing dependencies on HAS_DMA. This series allows to
build again "allmodconfig" and "allyesconfig" kernels tailored for
Sun-3, which sets NO_DMA=y.
Geert Uytterhoeven (6):
ata: SATA_MV should depend on HAS_DMA
ata: SATA_HIGHBANK should depend on HAS_DMA
ata: ATA_BMDMA should depend on HAS_DMA
ata: AHCI and other non-SFF native drivers should depend on HAS_DMA
libata: Make ata_sg_clean() static again
libata: Protect DMA core code by #ifdef CONFIG_HAS_DMA
drivers/ata/Kconfig | 7 ++++++
drivers/ata/libata-core.c | 61 +++++++++++++++++++++++++++--------------------
drivers/ata/libata.h | 1 -
3 files changed, 42 insertions(+), 27 deletions(-)
--
1.9.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH 1/6] ata: SATA_MV should depend on HAS_DMA
From: Geert Uytterhoeven @ 2017-01-03 18:09 UTC (permalink / raw)
To: Tejun Heo, Bartlomiej Zolnierkiewicz
Cc: linux-ide, linux-m68k, linux-kernel, Geert Uytterhoeven
In-Reply-To: <1483466989-9091-1-git-send-email-geert@linux-m68k.org>
If NO_DMA=y:
ERROR: "dma_pool_alloc" [drivers/ata/sata_mv.ko] undefined!
ERROR: "dmam_pool_create" [drivers/ata/sata_mv.ko] undefined!
ERROR: "dma_pool_free" [drivers/ata/sata_mv.ko] undefined!
Add a dependency on HAS_DMA to fix this.
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
drivers/ata/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index cbd101933c69322a..7b36b791078a4806 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -353,6 +353,7 @@ config SATA_HIGHBANK
config SATA_MV
tristate "Marvell SATA support"
+ depends on HAS_DMA
depends on PCI || ARCH_DOVE || ARCH_MV78XX0 || \
ARCH_MVEBU || ARCH_ORION5X || COMPILE_TEST
select GENERIC_PHY
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox