Re: new ata_port_operations for .pmp_{read,write} ?
Tejun Heo wrote: Mark Lord wrote: .. But no big deal. I can clone code and not bother you any more. In fact, some of the cloned code was already in sata_mv, and I removed it this past week in my local working copy. I'll just restore that, along with another big blob so that we can select pm port where needed. What a shame. The order is somewhat reversed here and I can understand why you're frustrated but I'm just trying to make things look right in long term, so feel free to bother me. :-) For temporary solution, I'm okay either way. I'll clean things up later when the necessary core changes are made. .. MMmm.. maybe the vendor unique FIS mechanism of the chipset can save the scenario here. It would seem to be a reasonable way to direct a FIS (anything up to 8KB) at a specific pmp, without changing the default pmp on the channel. I can have qc_issue use that mechanism for anything destined for pmp==15. If it works. The Marvell proprietary driver has some kludgey status polling wrapped around their own use of it. One of the chip errata apparently requires this anyway for doing the READ_LOG_EXT commands after a device error, so perhaps it will work out to be useful in more ways. Speaking of which.. I would like to sort out the freeze stuff, so that the sata_mv EH doesn't lock out the PMP commands as it does today. Can you recap what a LLD should be doing in the presence of a PM for EH purposes? Eg. media error on a PMP drive, so what core ATA functions should the sata_mv EH interrupt handler be calling, and in what sequence.. so that the libata-pmp/eh code can still succeed in it's queries to the PM? I think James is also interested in a decent explanation of how to tie into the libata-eh stuff. Like I noted before, sata_mv will need to eventually hard reset the channel regardless, but it does seem permitted to use PIO or vendor-unique-FIS PIO (with polling for either) in the interim. Thanks. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Hello, Mark. Mark Lord wrote: MMmm.. maybe the vendor unique FIS mechanism of the chipset can save the scenario here. It would seem to be a reasonable way to direct a FIS (anything up to 8KB) at a specific pmp, without changing the default pmp on the channel. I can have qc_issue use that mechanism for anything destined for pmp==15. If it works. The Marvell proprietary driver has some kludgey status polling wrapped around their own use of it. One of the chip errata apparently requires this anyway for doing the READ_LOG_EXT commands after a device error, so perhaps it will work out to be useful in more ways. Hmmm... Speaking of which.. I would like to sort out the freeze stuff, so that the sata_mv EH doesn't lock out the PMP commands as it does today. Can you recap what a LLD should be doing in the presence of a PM for EH purposes? Eg. media error on a PMP drive, so what core ATA functions should the sata_mv EH interrupt handler be calling, and in what sequence.. so that the libata-pmp/eh code can still succeed in it's queries to the PM? libata doesn't really put much restrictions on what a LLD should do on entering EH and if the controller's behavior is predictable, there's no reason to freeze the port. If the problem is that the DMA engine isn't usable after PMP error but it's known that the controller isn't gonna cause irq storm, no need to freeze. The only command EH issues before resetting which can use DMA protocol is READ_LOG_EXT. Maybe there needs to be a way to force PIO protocol for READ_LOG_EXT. Other than that, if no-data and PIO-only commands work fine, EH autopsy should work fine. Like I noted before, sata_mv will need to eventually hard reset the channel regardless, but it does seem permitted to use PIO or vendor-unique-FIS PIO (with polling for either) in the interim. Just set ATA_EH_HARDRESET (which will soon become ATA_EH_RESET with prefer-COMRESET patchset) from the interrupt handler before requesting EH. Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Tejun Heo wrote: libata doesn't really put much restrictions on what a LLD should do on entering EH and if the controller's behavior is predictable, there's no reason to freeze the port. If the problem is that the DMA engine isn't usable after PMP error but it's known that the controller isn't gonna cause irq storm, no need to freeze. The only command EH issues before resetting which can use DMA protocol is READ_LOG_EXT. Maybe there needs to be a way to force PIO protocol for READ_LOG_EXT. Other than that, if no-data and PIO-only commands work fine, EH autopsy should work fine. .. Eh? READ LOG EXT *is* a PIO command, as opposed to READ LOG DMA EXT which uses DMA. And the EH also issues PIO READ_BUFFER commands for PM ports, if a PM is present. ??? - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Hello, Mark. Mark Lord wrote: libata doesn't really put much restrictions on what a LLD should do on entering EH and if the controller's behavior is predictable, there's no reason to freeze the port. If the problem is that the DMA engine isn't usable after PMP error but it's known that the controller isn't gonna cause irq storm, no need to freeze. The only command EH issues before resetting which can use DMA protocol is READ_LOG_EXT. Maybe there needs to be a way to force PIO protocol for READ_LOG_EXT. Other than that, if no-data and PIO-only commands work fine, EH autopsy should work fine. .. Eh? READ LOG EXT *is* a PIO command, as opposed to READ LOG DMA EXT which uses DMA. Ah.. right. My bad. And the EH also issues PIO READ_BUFFER commands for PM ports, if a PM is present. H Where? -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Tejun Heo wrote: Hello, Mark. Mark Lord wrote: Mark Lord wrote: An alternative to all this, might be to expose the select_pmp() function shown in the sample code, and have libata-pmp.c call that, instead of having the new new .pmp_{read,write} functions. .. I wonder if this might be more viable than first thought. Say the LLD, be it ata_piix or sata_mv or sata_svw, were to provide an option ata_operations method for select_pmp_port(pmp), which the core could invoke prior to any direct manipulation of the shadow registers. I don't really think that's a good solution. That's the quickest solution for sata_mv but it just works around more fundamental problem of assuming SFF behavior in core layer which we need to drop anyway. I really would like to keep the LLD code small, and have good solid core routines for non-hardware specific functionality. All of this stuff I'm beginning to do with sata_mv would be trivial if I wanted to bloat the LLD, but really.. only a tiny bit of it need be custom to sata_mv. The existing SFF reset/probe/pmp stuff is just about exactly what sata_mv needs.. and I feel a strong desire to not clone/duplicate that hard-won functionality. I strongly agree but am having difficult time agreeing with the proposed approach. .. Okay, then. MASSIVE code-duplication, here I come! -ml - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Jeff Garzik wrote: Tejun Heo wrote: Yeah, exactly. I think what needs to be done is to separate out SFF assumptions from core layer, factor out SFF-proper helpers and use them to implement LLDs for quasi-SFF controllers. Thinking long term, I continue to hope that SFF support can eventually be separate into a sub-API, allowing people to disable (at compile time) SFF support completely if they don't need it. .. I can think of a few more pressing things to disable before that. Like ACPI and PMP. Modularizing those (or more crudely de-selecting them at build time) would save a lot of memory on small embedded boxes. Cheers - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Mark Lord wrote: Jeff Garzik wrote: Tejun Heo wrote: Yeah, exactly. I think what needs to be done is to separate out SFF assumptions from core layer, factor out SFF-proper helpers and use them to implement LLDs for quasi-SFF controllers. Thinking long term, I continue to hope that SFF support can eventually be separate into a sub-API, allowing people to disable (at compile time) SFF support completely if they don't need it. .. I can think of a few more pressing things to disable before that. Like ACPI and PMP. Modularizing those (or more crudely de-selecting them at build time) would save a lot of memory on small embedded boxes. Sure. Make these options dependent on CONFIG_EMBEDDED, and go to town from there... Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Tejun Heo wrote: Hello, Mark. Mark Lord wrote: Mark Lord wrote: An alternative to all this, might be to expose the select_pmp() function shown in the sample code, and have libata-pmp.c call that, instead of having the new new .pmp_{read,write} functions. .. I wonder if this might be more viable than first thought. Say the LLD, be it ata_piix or sata_mv or sata_vw, were to provide an option ata_operations method for select_pmp_port(pmp), which the core could invoke prior to any direct manipulation of the shadow registers. I don't really think that's a good solution. That's the quickest solution for sata_mv but it just works around more fundamental problem of assuming SFF behavior in core layer which we need to drop anyway. ... No, the quickest solution for sata_mv, the one I apparently will now be using, is to just clone about 250 lines of reset/debouce/probe code from libata-core and change perhaps five lines of it to work around this issue plus some chipset errata. What I was thinking of, before, is the SATA specification which details use of bits in the SCR to select a PMP port. Pretty much exactly as this chipset does it, except in the SCR rather than a proprieary port. But no big deal. I can clone code and not bother you any more. In fact, some of the cloned code was already in sata_mv, and I removed it this past week in my local working copy. I'll just restore that, along with another big blob so that we can select pm port where needed. What a shame. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
To repeat myself from threads past... In particular with the Marvell 6440 (SATA/SAS, drivers/scsi/mvsas.c) when SATA PMP support is completed, we will want to look at passing things other than normal ATA commands via -qc_issue. Although selection isn't necessarily, talking to a PMP is fundamentally an asynchronous, event-driven operation just like any other FIS conversation. We don't need to be adding hooks for each new and interesting type of FIS sent across the wire. That just leads to an awful API. Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Mark Lord wrote: No, the quickest solution for sata_mv, the one I apparently will now be using, is to just clone about 250 lines of reset/debouce/probe code from libata-core and change perhaps five lines of it to work around this issue plus some chipset errata. What I was thinking of, before, is the SATA specification which details use of bits in the SCR to select a PMP port. Pretty much exactly as this chipset does it, except in the SCR rather than a proprieary port. But no big deal. I can clone code and not bother you any more. In fact, some of the cloned code was already in sata_mv, and I removed it this past week in my local working copy. I'll just restore that, along with another big blob so that we can select pm port where needed. What a shame. The order is somewhat reversed here and I can understand why you're frustrated but I'm just trying to make things look right in long term, so feel free to bother me. :-) For temporary solution, I'm okay either way. I'll clean things up later when the necessary core changes are made. * Adding -pmp_select or -scr_pmp_rw or whatever callback to work around the current problem. * Duplicate code in sata_mv. Whichever way you go, can you please mark where it's different and why? Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Hello, Mark. Mark Lord wrote: Mark Lord wrote: An alternative to all this, might be to expose the select_pmp() function shown in the sample code, and have libata-pmp.c call that, instead of having the new new .pmp_{read,write} functions. .. I wonder if this might be more viable than first thought. Say the LLD, be it ata_piix or sata_mv or sata_svw, were to provide an option ata_operations method for select_pmp_port(pmp), which the core could invoke prior to any direct manipulation of the shadow registers. I don't really think that's a good solution. That's the quickest solution for sata_mv but it just works around more fundamental problem of assuming SFF behavior in core layer which we need to drop anyway. I really would like to keep the LLD code small, and have good solid core routines for non-hardware specific functionality. All of this stuff I'm beginning to do with sata_mv would be trivial if I wanted to bloat the LLD, but really.. only a tiny bit of it need be custom to sata_mv. The existing SFF reset/probe/pmp stuff is just about exactly what sata_mv needs.. and I feel a strong desire to not clone/duplicate that hard-won functionality. I strongly agree but am having difficult time agreeing with the proposed approach. Much of it I can see being shared with other half-and-half chipset drivers as we add PMP functionality to those. Do we have other chips which have PMP support but still uses mixed SFF interface? Maybe that's just the embedded side me showing through? It is tricky to define the right interfaces, though, as each chipset does throw its own unique curve balls at us. Yeah, exactly. I think what needs to be done is to separate out SFF assumptions from core layer, factor out SFF-proper helpers and use them to implement LLDs for quasi-SFF controllers. Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Tejun Heo wrote: Yeah, exactly. I think what needs to be done is to separate out SFF assumptions from core layer, factor out SFF-proper helpers and use them to implement LLDs for quasi-SFF controllers. Thinking long term, I continue to hope that SFF support can eventually be separate into a sub-API, allowing people to disable (at compile time) SFF support completely if they don't need it. Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Mark Lord wrote: Tejun, Here's a first cut at this for discussion. You may prefer different names for the invoking functions inside libata-pmp.c, rather than simply pmp_read() and pmp_write(), but I've been up too long and couldn't think of a better name. An alternative to all this, might be to expose the select_pmp() function shown in the sample code, and have libata-pmp.c call that, instead of having the new new .pmp_{read,write} functions. What do you think? * * * SATA host controllers which are not purely FIS-based require setup of a special register to select the active pmp for taskfile accesses. This can be done in the low-level driver for regular command issue on a link. But commands directed at a port-multiplier cause problems, because they leave the original link pmp de-selected afterwards. To fix this in a sane fashion, without tons of code duplication from libata into the low-level driver, it is necessary to be able to wrap the sata_pmp_read() and sata_pmp_write() functions. This patch provides two new struct ata_port_operations methods for this, and modifies the code in libata-pmp to use them if provided. ... Note that, while this does work for sata_mv, I'm still thinking about it. I'm not totally clear yet (more reading to do) as to how/when the ATA shadow/taskfile registers get updated to reflect those for the currently selected pmp.. It would seem that with other parts of libata-sff directly reading from the ctl, status, and altstatus registers during polling, command setup, and probing, that there might (?) be a loophole somewhere in this strategy. Is this scenario going to be possible: somebody calls sata_pmp_read() as part of, say, hotplug polling, and after that operation completes we then have code that calls ata_check_status() prior to the next tf_load / command issue ? If so, they'll see the wrong cached shadow status register. And for that matter, is it possible for sata_pmp_read() to be called while the link is active with another command ? Not today, it seems, but what about when hotplug polling gets implemented ? Mmm.. an amazing amount of complexity there for something that ought to be very simple. More reading to do, but comments from Tejun/others are welcomed. Thanks. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Mark Lord wrote: Note that, while this does work for sata_mv, I'm still thinking about it. I'm not totally clear yet (more reading to do) as to how/when the ATA shadow/taskfile registers get updated to reflect those for the currently selected pmp.. It would seem that with other parts of libata-sff directly reading from the ctl, status, and altstatus registers during polling, command setup, and probing, that there might (?) be a loophole somewhere in this strategy. Is this scenario going to be possible: somebody calls sata_pmp_read() as part of, say, hotplug polling, and after that operation completes we then have code that calls ata_check_status() prior to the next tf_load / command issue ? If so, they'll see the wrong cached shadow status register. .. Answering my own question here: the above scenario really doesn't matter. They'll see 0x50 in the status register, and be happy regardless. Because that's what should have been there before the command issue by sata_pmp_read() in the first place. So not-a-problem. And for that matter, is it possible for sata_pmp_read() to be called while the link is active with another command ? Not today, it seems, but what about when hotplug polling gets implemented ? .. That's the one I'm most concerned about. Should I be? -ml - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Mark Lord wrote: Mark Lord wrote: ... And for that matter, is it possible for sata_pmp_read() to be called while the link is active with another command ? Not today, it seems, but what about when hotplug polling gets implemented ? .. That's the one I'm most concerned about. Should I be? ... Tejun, On a related note, I'm now looking into PMP error handling in the driver. The obvious thing I see that I want to fix, is that after a media error on any PMP attached drive, I get this: ata20.00: failed to read SCR 1 (Emask=0x40) ata20.01: failed to read SCR 1 (Emask=0x40) ata20.02: failed to read SCR 1 (Emask=0x40) ata20.03: failed to read SCR 1 (Emask=0x40) ata20.04: failed to read SCR 1 (Emask=0x40) Okay, so those are from sata_pmp_read(), which cannot even issue it's commands because the port was frozen by the EH. Is this expected? I'm not entirely clear what to do in the EH for this driver. The chipset docs say that after just about any kind of error software must do a hard reset of the channel to make it usable again. But I suspect that PIO commands may be okay before that, and sata_pmp_read() is trying to issue a PIO command. How to resolve this? - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Hello, Mark. Mark Lord wrote: This patch provides two new struct ata_port_operations methods for this, and modifies the code in libata-pmp to use them if provided. ... Note that, while this does work for sata_mv, I'm still thinking about it. I'm not totally clear yet (more reading to do) as to how/when the ATA shadow/taskfile registers get updated to reflect those for the currently selected pmp.. It would seem that with other parts of libata-sff directly reading from the ctl, status, and altstatus registers during polling, command setup, and probing, that there might (?) be a loophole somewhere in this strategy. Yeah, this is an interesting problem. There are basically multiple sets of TF registers and the SFF way of assuming single set doesn't really work well and I don't really think adding -pmp_read/write is the correct long term solution to the problem. We need to confine direct TF register accesses to SFF layer only as controllers which don't implement SFF interface may or may not emulate TF registers and even when they do, it sometimes can't really match the SFF behavior. For command execution, TF write is already performed by qc_prep and issue and TF read back should be performed by LLD if RESULT_TF is set. For EH, the reset methods should be responsible whether or how the TF registers are accessed. Mark, for your case, I think the correct thing to do is to factor out the necessary bare-bone part from SFF implementation and use it with necessary PMP register setup once the necessary change is made to the core layer. The controller is NOT a SFF one and I don't think stretching SFF implementation to fit mv's PMP-aware emulation of it is a good idea. Maybe we can fit libata to it but it's likely we'll need to modify it again to fit different emulation from different vendor. Is this scenario going to be possible: somebody calls sata_pmp_read() as part of, say, hotplug polling, and after that operation completes we then have code that calls ata_check_status() prior to the next tf_load / command issue ? If so, they'll see the wrong cached shadow status register. I think what should happen is that any of TF registers isn't accessed behind LLD's back. Then, you don't have nothing to worry about. If the controller emulates some of SFF interface, you can always properly wrap SFF helpers with necessary setup and teardown added. Mmm.. an amazing amount of complexity there for something that ought to be very simple. I wish things are a bit clearer now. I think the problem here is that we're assuming SFF TF access on controllers which aren't really SFF. For sil24 and ahci, the driver emulates it and it isn't too difficult. The picture gets more interesting for sata_mv as its hardware interface much closer to SFF than sil24 or ahci and TF registers matter much more. For ahci and sil24, LLD can just fool libata core layer which assumes ubiquitous TF access. TF registers don't really matter to controller operation anyway and feeding bogus values work well. For sata_mv, it's different. Those registers are integral part of controller operation and sata_mv can't really tolerate core layer stepping in w/o notifying LLD. Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Hello, Mark. Mark Lord wrote: And for that matter, is it possible for sata_pmp_read() to be called while the link is active with another command ? Not today, it seems, but what about when hotplug polling gets implemented ? .. That's the one I'm most concerned about. Should I be? ... Tejun, On a related note, I'm now looking into PMP error handling in the driver. The obvious thing I see that I want to fix, is that after a media error on any PMP attached drive, I get this: ata20.00: failed to read SCR 1 (Emask=0x40) ata20.01: failed to read SCR 1 (Emask=0x40) ata20.02: failed to read SCR 1 (Emask=0x40) ata20.03: failed to read SCR 1 (Emask=0x40) ata20.04: failed to read SCR 1 (Emask=0x40) Okay, so those are from sata_pmp_read(), which cannot even issue it's commands because the port was frozen by the EH. Hmm.. media error causes freeze? Anyways, yeah, those are expected if the port is frozen. Those are from link autopsy. Maybe it's better to skip SCR access on fan-out ports if host port is frozen or at least suppress the messages. Is this expected? I'm not entirely clear what to do in the EH for this driver. The chipset docs say that after just about any kind of error software must do a hard reset of the channel to make it usable again. But I suspect that PIO commands may be okay before that, and sata_pmp_read() is trying to issue a PIO command. If the controller needs to be frozen after any kind of error, I don't think there's much left to do other than suppressing those annoying messages. Hmmm.. how does the controller handle ATAPI check sense? -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Tejun Heo wrote: Yeah, this is an interesting problem. There are basically multiple sets of TF registers and the SFF way of assuming single set doesn't really work well and I don't really think adding -pmp_read/write is the correct long term solution to the problem. We need to confine direct TF register accesses to SFF layer only as controllers which don't implement SFF interface may or may not emulate TF registers and even when they do, it sometimes can't really match the SFF behavior. Strongly agreed. I wish things are a bit clearer now. I think the problem here is that we're assuming SFF TF access on controllers which aren't really SFF. For sil24 and ahci, the driver emulates it and it isn't too difficult. The picture gets more interesting for sata_mv as its hardware interface much closer to SFF than sil24 or ahci and TF registers matter much more. For ahci and sil24, LLD can just fool libata core layer which assumes ubiquitous TF access. TF registers don't really matter to controller operation anyway and feeding bogus values work well. For sata_mv, it's different. Those registers are integral part of controller operation and sata_mv can't really tolerate core layer stepping in w/o notifying LLD. I would definitely like to move away from the model where non-SFF drivers have to emulate SFF in any way. Re-reviewing the code, I don't see a lot of TF accesses outside of SFF-specific code, so we are already in pretty good shape. I think the picture gets more complicated with sata_mv and similar drivers (soon sata_svw, too), because they use the SFF code but only for certain TF protocols. The emulation question is not as clear, with those drivers. Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new ata_port_operations for .pmp_{read,write} ?
Mark Lord wrote: An alternative to all this, might be to expose the select_pmp() function shown in the sample code, and have libata-pmp.c call that, instead of having the new new .pmp_{read,write} functions. .. I wonder if this might be more viable than first thought. Say the LLD, be it ata_piix or sata_mv or sata_svw, were to provide an option ata_operations method for select_pmp_port(pmp), which the core could invoke prior to any direct manipulation of the shadow registers. It would really only be needed in perhaps three or four places total (eg. sata_pmp_read/write, and before writes to the ata ctl register). This could be a reasonably tidy way for the core to keep the LLD abreast of its intent when accessing things. I really would like to keep the LLD code small, and have good solid core routines for non-hardware specific functionality. All of this stuff I'm beginning to do with sata_mv would be trivial if I wanted to bloat the LLD, but really.. only a tiny bit of it need be custom to sata_mv. The existing SFF reset/probe/pmp stuff is just about exactly what sata_mv needs.. and I feel a strong desire to not clone/duplicate that hard-won functionality. Much of it I can see being shared with other half-and-half chipset drivers as we add PMP functionality to those. Maybe that's just the embedded side me showing through? It is tricky to define the right interfaces, though, as each chipset does throw its own unique curve balls at us. I may prototype this select_pmp_port() concept in sata_mv(), and see how it works out (or not). Cheers - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html