Message ID | 1374851518-4802-1-git-send-email-mark.langsdorf@calxeda.com |
---|---|
State | Not Applicable |
Delegated to: | David Miller |
Headers | show |
On Fri, Jul 26, 2013 at 10:11:54AM -0500, Mark Langsdorf wrote: > The ACTIVITY and ERROR signals were reversed in the original commit. > Fix that. And what's the implication of this change? Does this change the behavior of the driver? If so, how? If not, why not? *Please* be more descriptive in patch descriptions and explain *why* the changes are being made. Thanks.
On 07/29/2013 11:55 AM, Tejun Heo wrote: > On Fri, Jul 26, 2013 at 10:11:54AM -0500, Mark Langsdorf wrote: >> The ACTIVITY and ERROR signals were reversed in the original commit. >> Fix that. > > And what's the implication of this change? Does this change the > behavior of the driver? If so, how? If not, why not? *Please* be > more descriptive in patch descriptions and explain *why* the changes > are being made. I didn't think I needed more of an explanation than "this is completely wrong per the spec" but I'll revise and resubmit. --Mark Langsdorf Calxeda, Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On Fri, Aug 02, 2013 at 10:23:49AM -0500, Mark Langsdorf wrote: > I didn't think I needed more of an explanation than "this is completely > wrong per the spec" but I'll revise and resubmit. Of course you always need to explain the implications as the result of "being completely wrong per the spec" can range from no actual effect at all to massive data corruption. In general, it's a good idea for a patch descrpition to describe why the change is an improvement and how it changes the behavior. The former provides the rationale for inclusion and the latter helps various people including the subsystem, -stable and distro kernel maintainers to assess the risk involved in taking the change. Thanks.
diff --git a/drivers/ata/sata_highbank.c b/drivers/ata/sata_highbank.c index d047d92..e9a4f46 100644 --- a/drivers/ata/sata_highbank.c +++ b/drivers/ata/sata_highbank.c @@ -86,11 +86,11 @@ struct ecx_plat_data { #define SGPIO_SIGNALS 3 #define ECX_ACTIVITY_BITS 0x300000 -#define ECX_ACTIVITY_SHIFT 2 +#define ECX_ACTIVITY_SHIFT 0 #define ECX_LOCATE_BITS 0x80000 #define ECX_LOCATE_SHIFT 1 #define ECX_FAULT_BITS 0x400000 -#define ECX_FAULT_SHIFT 0 +#define ECX_FAULT_SHIFT 2 static inline int sgpio_bit_shift(struct ecx_plat_data *pdata, u32 port, u32 shift) {
The ACTIVITY and ERROR signals were reversed in the original commit. Fix that. Signed-off-by: Mark Langsdorf <mark.langsdorf@calxeda.com> --- drivers/ata/sata_highbank.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)