SATA Framework Port Multiplier Support [PSARC/2009/394 Self Review]
+1 On 07/14/09 07:56 PM, Alan Perry wrote: Following the recommendation of Garrett D'Amore and James Carlson, this is being promoted from Self-Review to Fast-Track. The timer expires on 21 July 2009. alan Alan Perry wrote: I am sponsoring this self-review case. This case documents additional changes to an existing case. I believe that this case qualifies for self-review because the interfaces are Consolidation Private and backwards compatible with the existing interfaces. Template Version: @(#)sac_nextcase 1.68 02/23/09 SMI This information is Copyright 2009 Sun Microsystems 1. Introduction 1.1. Project/Component Working Name: SATA Framework Port Multiplier Support 1.2. Name of Document Author/Supplier: Author: Xiaoyu Zhang 1.3 Date of This Document: 14 July, 2009 4. Technical Description -- - Rick Matthews email: Rick.Matthews at sun.com Sun Microsystems, Inc. phone:+1(651) 554-1518 1270 Eagan Industrial Road phone(internal): 54418 Suite 160 fax: +1(651) 554-1540 Eagan, MN 55121-1231 USAmain: +1(651) 554-1500 -
SATA Framework Port Multiplier Support [PSARC/2009/394 Self Review]
Btw, this case was approved at PSARC today. Alan, can you update the IAM file please? Thanks. - Garrett Rick Matthews wrote: +1 On 07/14/09 07:56 PM, Alan Perry wrote: Following the recommendation of Garrett D'Amore and James Carlson, this is being promoted from Self-Review to Fast-Track. The timer expires on 21 July 2009. alan Alan Perry wrote: I am sponsoring this self-review case. This case documents additional changes to an existing case. I believe that this case qualifies for self-review because the interfaces are Consolidation Private and backwards compatible with the existing interfaces. Template Version: @(#)sac_nextcase 1.68 02/23/09 SMI This information is Copyright 2009 Sun Microsystems 1. Introduction 1.1. Project/Component Working Name: SATA Framework Port Multiplier Support 1.2. Name of Document Author/Supplier: Author: Xiaoyu Zhang 1.3 Date of This Document: 14 July, 2009 4. Technical Description
SATA Framework Port Multiplier Support [PSARC/2009/394 Self Review]
Alan Perry wrote: James Carlson wrote: Alan Perry wrote: Garrett D'Amore wrote: This is not the only criteria for self-review. Self-review cases must also be so obvious and self explanatory that no further review is desired or required. They should usually should not be introducing new architecture. I believe this case exceed this threshold, and I would like to make sure it is properly reviewed. Please convert this to a fast track with a one week timer. This case does not introduce new architecture. It is making changes to an architecture introduced in PSARC/2004/779. As far as the obvious criteria, obvious to whom? Obvious to the reviewers -- in this case, that would be those on psarc-ext, such as Garrett. I think his response was completely appropriate. It wasn't in any way a denial of anything the project team has done; it was simply a request to have a couple of days to look the materials over before declaring the ARC review of the change to be complete. Reviews outside of the ARC are a great thing, and it's good to know that the project team has sought such reviews, and that there are willing people with domain expertise available to provide them. They're never a substitute for open ARC review, though. Does this mean that any case that requires specific knowledge of a particular technology is now not eligible for self-review because PSARC reviewers who does not work on that technology are unlikely to find the case obvious? Possibly, but not necessarily. Just because you find a case obvious doesn't meant that you have covered all of the concerns. Its not fair to assume that nobody outside of the project can provide input though, and if there is *any* doubt then fast track is always safer than auto-approval. Excuse me while I express some frustration here. In the past, I have sponsored cases with more substantial changes and have been asked why I was wasting people's time by submitting a fast-track and not a self-review. This requires human judgment. If you aren't sure, it would never hurt to ask a member for an opinion about the right level of review. Established architecture would be cases where you add no new interfaces, or only project private interfaces, and the plumbing is obvious. Adding new interfaces for device drivers to call and use definitely exceeds that the threshold and IMO constitutes new architecture, and even if it seems obvious to the project team it needs to be given the opportunity for people outside the project team (who may or may not be ARC members) to provide feedback. -- Garrett
SATA Framework Port Multiplier Support [PSARC/2009/394 Self Review]
Just to be clear, I do not have a problem with more review. However, I am concerned about inconsistent application of the documented process. Garrett D'Amore wrote: Excuse me while I express some frustration here. In the past, I have sponsored cases with more substantial changes and have been asked why I was wasting people's time by submitting a fast-track and not a self-review. This requires human judgment. If you aren't sure, it would never hurt to ask a member for an opinion about the right level of review. Based on my previous experience with cases submitted before, I was sure. Otherwise I would have submitted a fast-track. Established architecture would be cases where you add no new interfaces, or only project private interfaces, and the plumbing is obvious. Adding new interfaces for device drivers to call and use definitely exceeds that the threshold and IMO constitutes new architecture, and even if it seems obvious to the project team it needs to be given the opportunity for people outside the project team (who may or may not be ARC members) to provide feedback. These interfaces are consolidation private interfaces that are used only by the sata development team. According to the Interface Taxonomy, ARC review of the specs for consolidation private work is Not necessary. This document also says If a Consolidation Private interface is reviewed by the ARC, ask that ARC if they want to review later changes to that interface. Based on previously submitted cases, the answer seemed to be 'no' for minor changes to the interface. alan
SATA Framework Port Multiplier Support [PSARC/2009/394 Self Review]
John Plocher wrote:
On Wed, Jul 15, 2009 at 10:17 AM, Alan PerryAlan.Perry at sun.com wrote:
However, I am concerned about inconsistent application of the documented
process.
In the past, my decision tree looked like this:
Proposed stability level for new interfaces:
{Project Private, Not an Interface} ) = Self Review
{Consolidation Private} = Fast Track
{Sun Private} = Deny with extreme prejudice :-)
else
= Fasttrack or full case, as circumstances require, based on
whether incompatible changes are being made to interfaces
that have existing expectations of longevity...
That sounds reasonable.
However, as I have noted, in the past I have sponsored cases with more
significant changes where PSARC members said why is this a fast-track
and not a self-review.
I like your decision tree, but it isn't what everyone does and it
doesn't completely map to the documented process.
alan
SATA Framework Port Multiplier Support [PSARC/2009/394 Self Review]
John Plocher wrote: On Wed, Jul 15, 2009 at 11:02 AM, Alan PerryAlan.Perry at sun.com wrote: sponsored cases with more significant changes where PSARC members said why is this a fast-track and not a self-review. The key point isn't significant changes, but rather what is the existing stability level of the things being changed? - we do ARC stuff so we can manage the impact and repercussions of the stuff we change. Adding things is easy, as is changing things in compatible ways. Even incompatible changes are easy, as long as they are to things that have low longevity/stability expectations. As you move up the stability levels with incompatible changes, the need for review naturally increases, because the side effects of such changes impacts more and more projects/teams. So maybe the best litmus test is one that captures the difficulty of managing the change once it gets out into the world: who might be negatively impacted by my change? - with none but me equating to self review, family and friends, but we can easily deal with it to fast track and people I don't know well to full review. If this is the case, could the materials that describe the process be updated to reflect this? I am specifically referring to the Self-Review Duties and Self Review Process pages on the sac.eng website as well as the Interface Taxonomy, which says that ARC review is not necessary for Consolidation Private interfaces. alan
SATA Framework Port Multiplier Support [PSARC/2009/394 Self Review]
Alan Perry wrote: I am sponsoring this self-review case. This case documents additional changes to an existing case. I believe that this case qualifies for self-review because the interfaces are Consolidation Private and backwards compatible with the existing interfaces. This is not the only criteria for self-review. Self-review cases must also be so obvious and self explanatory that no further review is desired or required. They should usually should not be introducing new architecture. I believe this case exceed this threshold, and I would like to make sure it is properly reviewed. Please convert this to a fast track with a one week timer. -- Garrett Template Version: @(#)sac_nextcase 1.68 02/23/09 SMI This information is Copyright 2009 Sun Microsystems 1. Introduction 1.1. Project/Component Working Name: SATA Framework Port Multiplier Support 1.2. Name of Document Author/Supplier: Author: Xiaoyu Zhang 1.3 Date of This Document: 14 July, 2009 4. Technical Description 4.1. Background --- PSARC/2004/779 [1] established the SATA HBA Framework interface. PSARC/2005/679 [2] expanded this interface as needed to suport the marvell88sx and si3124 SATA HBA drivers. PSARC 2007/274 [3] expanded the interface further to support Native Command Queuing (NCQ) and ATAPI devices. PSARC/2007/448 [4] expanded the interface again to support releasing DMA framework-allocated resources associated with a command's buffer. This fast-track describes further interface changes required to support SATA port multipliers. 4.2. The Problem 4.2.1. Required to support READ/WRITE PORTMULT command -- According to the SATA Specification 2.6 [5] and the Port Multiplier Specification [6], READ PORT MULTIPLIER and WRITE PORT MULIPLIER are used to access the registers of the port multiplier. These two commands are necessary to the successful enumeration of the the port multiplier. Both SATA HBA drivers as well as the SATA module have internal functions operating on sata_pkt rather than isolated SATA commands. Therefore, READ/WRITE PORTMULT commands should be delivered to SATA HBA drivers in sata_pkt structures. 4.2.2. The insufficient sata_device interface - The sata_device is used as a parameter to the HBA probe entry point. Existing HBA implementations update only SATA Control Registers values in response to probe port operation. The port multiplier has its own Global Status Control Registers, in which the parameters and status of the port multiplier itself are stored. The sata module needs this register information, so there needs to be a method for the SATA HBA driver to provide this information. 4.2.3. Required to handle port multiplier quirks The SATA specification 2.6 [5] defines the registers and enumeration process for port multipliers, however, some existing port multiplier models have quirks and might break the general enumeration or initialization process. The HBA driver should have idea of these quirks and determine its reaction. Additions to the interface are required to handle Port Multiplier behavior that is different the specification. 4.3. The Proposal - 4.3.1. Summary -- Revise the SATA interface to support port multiplier. Port multiplier support was partially designed and implemented inside SATA module. The proposal defines new interface functions necessary to support new SATA commands and new fields in sata_device structure to support port multiplier device. A blacklist structure is defined is added to to dealing with port multiplier discrepancies from the SATA specification. The initial consumer of the modified interface will be AHCI driver implementing SATA port multiplier support. Since the existing SATA HBA drivers and SATA module do not currently implement the structure version checking (except for sata_hba_tran structure), the proposed change will maintain the binary compatibility of the modified interface structure (sata_device). A SATA module supporting these interface changes will operate with SATA HBA drivers using the unmodified interface as well as SATA HBA drivers using the modified interface. 4.3.2. Interface Modifications -- a) Add new structure sata_gscr. (See section 5.2.1 for more details) b) Modified sata_device structure. Add new field satadev_pmult_gscr for port multiplier device. (See section 5.3.1 for more details) c) The sata_device structure version (SATA_DEVICE_REV) will be increased to indicate added functionality. Current veriosn level is 1 - it will be increased to 2. (See section 5.3.1 for more details). d) The sata_hba_tran structure version (SATA_TRAN_HBA_REV) will be increased to 3 to
SATA Framework Port Multiplier Support [PSARC/2009/394 Self Review]
Garrett D'Amore wrote: Alan Perry wrote: I am sponsoring this self-review case. This case documents additional changes to an existing case. I believe that this case qualifies for self-review because the interfaces are Consolidation Private and backwards compatible with the existing interfaces. This is not the only criteria for self-review. Self-review cases must also be so obvious and self explanatory that no further review is desired or required. They should usually should not be introducing new architecture. I believe this case exceed this threshold, and I would like to make sure it is properly reviewed. Please convert this to a fast track with a one week timer. The case has been reviewed by at least two of the engineers that work on the sata framework, including the owner of the framework, and presented to the entire sata dev team. Who would review this case outside of the sata dev team? alan -- Garrett Template Version: @(#)sac_nextcase 1.68 02/23/09 SMI This information is Copyright 2009 Sun Microsystems 1. Introduction 1.1. Project/Component Working Name: SATA Framework Port Multiplier Support 1.2. Name of Document Author/Supplier: Author: Xiaoyu Zhang 1.3 Date of This Document: 14 July, 2009 4. Technical Description 4.1. Background --- PSARC/2004/779 [1] established the SATA HBA Framework interface. PSARC/2005/679 [2] expanded this interface as needed to suport the marvell88sx and si3124 SATA HBA drivers. PSARC 2007/274 [3] expanded the interface further to support Native Command Queuing (NCQ) and ATAPI devices. PSARC/2007/448 [4] expanded the interface again to support releasing DMA framework-allocated resources associated with a command's buffer. This fast-track describes further interface changes required to support SATA port multipliers. 4.2. The Problem 4.2.1. Required to support READ/WRITE PORTMULT command -- According to the SATA Specification 2.6 [5] and the Port Multiplier Specification [6], READ PORT MULTIPLIER and WRITE PORT MULIPLIER are used to access the registers of the port multiplier. These two commands are necessary to the successful enumeration of the the port multiplier. Both SATA HBA drivers as well as the SATA module have internal functions operating on sata_pkt rather than isolated SATA commands. Therefore, READ/WRITE PORTMULT commands should be delivered to SATA HBA drivers in sata_pkt structures. 4.2.2. The insufficient sata_device interface - The sata_device is used as a parameter to the HBA probe entry point. Existing HBA implementations update only SATA Control Registers values in response to probe port operation. The port multiplier has its own Global Status Control Registers, in which the parameters and status of the port multiplier itself are stored. The sata module needs this register information, so there needs to be a method for the SATA HBA driver to provide this information. 4.2.3. Required to handle port multiplier quirks The SATA specification 2.6 [5] defines the registers and enumeration process for port multipliers, however, some existing port multiplier models have quirks and might break the general enumeration or initialization process. The HBA driver should have idea of these quirks and determine its reaction. Additions to the interface are required to handle Port Multiplier behavior that is different the specification. 4.3. The Proposal - 4.3.1. Summary -- Revise the SATA interface to support port multiplier. Port multiplier support was partially designed and implemented inside SATA module. The proposal defines new interface functions necessary to support new SATA commands and new fields in sata_device structure to support port multiplier device. A blacklist structure is defined is added to to dealing with port multiplier discrepancies from the SATA specification. The initial consumer of the modified interface will be AHCI driver implementing SATA port multiplier support. Since the existing SATA HBA drivers and SATA module do not currently implement the structure version checking (except for sata_hba_tran structure), the proposed change will maintain the binary compatibility of the modified interface structure (sata_device). A SATA module supporting these interface changes will operate with SATA HBA drivers using the unmodified interface as well as SATA HBA drivers using the modified interface. 4.3.2. Interface Modifications -- a) Add new structure sata_gscr. (See section 5.2.1 for more details) b) Modified sata_device structure. Add new field satadev_pmult_gscr for port multiplier device. (See section 5.3.1 for
SATA Framework Port Multiplier Support [PSARC/2009/394 Self Review]
Garrett D'Amore wrote: Alan Perry wrote: I am sponsoring this self-review case. This case documents additional changes to an existing case. I believe that this case qualifies for self-review because the interfaces are Consolidation Private and backwards compatible with the existing interfaces. This is not the only criteria for self-review. Self-review cases must also be so obvious and self explanatory that no further review is desired or required. They should usually should not be introducing new architecture. I believe this case exceed this threshold, and I would like to make sure it is properly reviewed. Please convert this to a fast track with a one week timer. This case does not introduce new architecture. It is making changes to an architecture introduced in PSARC/2004/779. As far as the obvious criteria, obvious to whom? I find the interface changes fairly obvious. I did have to read about how SATA port multipliers work first and think about ways to implement support for them across the sata framework/sata HBA driver split. alan -- Garrett Template Version: @(#)sac_nextcase 1.68 02/23/09 SMI This information is Copyright 2009 Sun Microsystems 1. Introduction 1.1. Project/Component Working Name: SATA Framework Port Multiplier Support 1.2. Name of Document Author/Supplier: Author: Xiaoyu Zhang 1.3 Date of This Document: 14 July, 2009 4. Technical Description 4.1. Background --- PSARC/2004/779 [1] established the SATA HBA Framework interface. PSARC/2005/679 [2] expanded this interface as needed to suport the marvell88sx and si3124 SATA HBA drivers. PSARC 2007/274 [3] expanded the interface further to support Native Command Queuing (NCQ) and ATAPI devices. PSARC/2007/448 [4] expanded the interface again to support releasing DMA framework-allocated resources associated with a command's buffer. This fast-track describes further interface changes required to support SATA port multipliers. 4.2. The Problem 4.2.1. Required to support READ/WRITE PORTMULT command -- According to the SATA Specification 2.6 [5] and the Port Multiplier Specification [6], READ PORT MULTIPLIER and WRITE PORT MULIPLIER are used to access the registers of the port multiplier. These two commands are necessary to the successful enumeration of the the port multiplier. Both SATA HBA drivers as well as the SATA module have internal functions operating on sata_pkt rather than isolated SATA commands. Therefore, READ/WRITE PORTMULT commands should be delivered to SATA HBA drivers in sata_pkt structures. 4.2.2. The insufficient sata_device interface - The sata_device is used as a parameter to the HBA probe entry point. Existing HBA implementations update only SATA Control Registers values in response to probe port operation. The port multiplier has its own Global Status Control Registers, in which the parameters and status of the port multiplier itself are stored. The sata module needs this register information, so there needs to be a method for the SATA HBA driver to provide this information. 4.2.3. Required to handle port multiplier quirks The SATA specification 2.6 [5] defines the registers and enumeration process for port multipliers, however, some existing port multiplier models have quirks and might break the general enumeration or initialization process. The HBA driver should have idea of these quirks and determine its reaction. Additions to the interface are required to handle Port Multiplier behavior that is different the specification. 4.3. The Proposal - 4.3.1. Summary -- Revise the SATA interface to support port multiplier. Port multiplier support was partially designed and implemented inside SATA module. The proposal defines new interface functions necessary to support new SATA commands and new fields in sata_device structure to support port multiplier device. A blacklist structure is defined is added to to dealing with port multiplier discrepancies from the SATA specification. The initial consumer of the modified interface will be AHCI driver implementing SATA port multiplier support. Since the existing SATA HBA drivers and SATA module do not currently implement the structure version checking (except for sata_hba_tran structure), the proposed change will maintain the binary compatibility of the modified interface structure (sata_device). A SATA module supporting these interface changes will operate with SATA HBA drivers using the unmodified interface as well as SATA HBA drivers using the modified interface. 4.3.2. Interface Modifications -- a) Add new structure sata_gscr. (See section 5.2.1 for
SATA Framework Port Multiplier Support [PSARC/2009/394 Self Review]
Alan Perry wrote: Garrett D'Amore wrote: This is not the only criteria for self-review. Self-review cases must also be so obvious and self explanatory that no further review is desired or required. They should usually should not be introducing new architecture. I believe this case exceed this threshold, and I would like to make sure it is properly reviewed. Please convert this to a fast track with a one week timer. This case does not introduce new architecture. It is making changes to an architecture introduced in PSARC/2004/779. As far as the obvious criteria, obvious to whom? Obvious to the reviewers -- in this case, that would be those on psarc-ext, such as Garrett. I think his response was completely appropriate. It wasn't in any way a denial of anything the project team has done; it was simply a request to have a couple of days to look the materials over before declaring the ARC review of the change to be complete. Reviews outside of the ARC are a great thing, and it's good to know that the project team has sought such reviews, and that there are willing people with domain expertise available to provide them. They're never a substitute for open ARC review, though.
SATA Framework Port Multiplier Support [PSARC/2009/394 Self Review]
James Carlson wrote: Alan Perry wrote: Garrett D'Amore wrote: This is not the only criteria for self-review. Self-review cases must also be so obvious and self explanatory that no further review is desired or required. They should usually should not be introducing new architecture. I believe this case exceed this threshold, and I would like to make sure it is properly reviewed. Please convert this to a fast track with a one week timer. This case does not introduce new architecture. It is making changes to an architecture introduced in PSARC/2004/779. As far as the obvious criteria, obvious to whom? Obvious to the reviewers -- in this case, that would be those on psarc-ext, such as Garrett. I think his response was completely appropriate. It wasn't in any way a denial of anything the project team has done; it was simply a request to have a couple of days to look the materials over before declaring the ARC review of the change to be complete. Reviews outside of the ARC are a great thing, and it's good to know that the project team has sought such reviews, and that there are willing people with domain expertise available to provide them. They're never a substitute for open ARC review, though. Does this mean that any case that requires specific knowledge of a particular technology is now not eligible for self-review because PSARC reviewers who does not work on that technology are unlikely to find the case obvious? Excuse me while I express some frustration here. In the past, I have sponsored cases with more substantial changes and have been asked why I was wasting people's time by submitting a fast-track and not a self-review. alan
SATA Framework Port Multiplier Support [PSARC/2009/394 Self Review]
Following the recommendation of Garrett D'Amore and James Carlson, this is being promoted from Self-Review to Fast-Track. The timer expires on 21 July 2009. alan Alan Perry wrote: I am sponsoring this self-review case. This case documents additional changes to an existing case. I believe that this case qualifies for self-review because the interfaces are Consolidation Private and backwards compatible with the existing interfaces. Template Version: @(#)sac_nextcase 1.68 02/23/09 SMI This information is Copyright 2009 Sun Microsystems 1. Introduction 1.1. Project/Component Working Name: SATA Framework Port Multiplier Support 1.2. Name of Document Author/Supplier: Author: Xiaoyu Zhang 1.3 Date of This Document: 14 July, 2009 4. Technical Description 4.1. Background --- PSARC/2004/779 [1] established the SATA HBA Framework interface. PSARC/2005/679 [2] expanded this interface as needed to suport the marvell88sx and si3124 SATA HBA drivers. PSARC 2007/274 [3] expanded the interface further to support Native Command Queuing (NCQ) and ATAPI devices. PSARC/2007/448 [4] expanded the interface again to support releasing DMA framework-allocated resources associated with a command's buffer. This fast-track describes further interface changes required to support SATA port multipliers. 4.2. The Problem 4.2.1. Required to support READ/WRITE PORTMULT command -- According to the SATA Specification 2.6 [5] and the Port Multiplier Specification [6], READ PORT MULTIPLIER and WRITE PORT MULIPLIER are used to access the registers of the port multiplier. These two commands are necessary to the successful enumeration of the the port multiplier. Both SATA HBA drivers as well as the SATA module have internal functions operating on sata_pkt rather than isolated SATA commands. Therefore, READ/WRITE PORTMULT commands should be delivered to SATA HBA drivers in sata_pkt structures. 4.2.2. The insufficient sata_device interface - The sata_device is used as a parameter to the HBA probe entry point. Existing HBA implementations update only SATA Control Registers values in response to probe port operation. The port multiplier has its own Global Status Control Registers, in which the parameters and status of the port multiplier itself are stored. The sata module needs this register information, so there needs to be a method for the SATA HBA driver to provide this information. 4.2.3. Required to handle port multiplier quirks The SATA specification 2.6 [5] defines the registers and enumeration process for port multipliers, however, some existing port multiplier models have quirks and might break the general enumeration or initialization process. The HBA driver should have idea of these quirks and determine its reaction. Additions to the interface are required to handle Port Multiplier behavior that is different the specification. 4.3. The Proposal - 4.3.1. Summary -- Revise the SATA interface to support port multiplier. Port multiplier support was partially designed and implemented inside SATA module. The proposal defines new interface functions necessary to support new SATA commands and new fields in sata_device structure to support port multiplier device. A blacklist structure is defined is added to to dealing with port multiplier discrepancies from the SATA specification. The initial consumer of the modified interface will be AHCI driver implementing SATA port multiplier support. Since the existing SATA HBA drivers and SATA module do not currently implement the structure version checking (except for sata_hba_tran structure), the proposed change will maintain the binary compatibility of the modified interface structure (sata_device). A SATA module supporting these interface changes will operate with SATA HBA drivers using the unmodified interface as well as SATA HBA drivers using the modified interface. 4.3.2. Interface Modifications -- a) Add new structure sata_gscr. (See section 5.2.1 for more details) b) Modified sata_device structure. Add new field satadev_pmult_gscr for port multiplier device. (See section 5.3.1 for more details) c) The sata_device structure version (SATA_DEVICE_REV) will be increased to indicate added functionality. Current veriosn level is 1 - it will be increased to 2. (See section 5.3.1 for more details). d) The sata_hba_tran structure version (SATA_TRAN_HBA_REV) will be increased to 3 to indicate new functionality level of the entire SATA framework interface. (See section 5.3.2 for more details) e) Add three new SATA module interface functions. + sata_get_rdwr_pmult_pkt(); + sata_free_rdwr_pmult_pkt();
