Re: [PATCH 1/2] Add support for Designware SATA controller driver
Benjamin Herrenschmidt wrote: Patch 2/2 is just adding the device-tree bits to the platform for this driver to match against, so it's no big deal. However, Feng, we already mentioned that you do a lot of unrelated changes to the Kconfig file that shouldn't be part of this patch, just add the entry for this chip, don't move stuff around in Kconfig, unless I missed a good reason why this is done that way. Quite correct -- those drivers/ata/Kconfig changes are bogus. Jeff ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2] Add support for Designware SATA controller driver
On Fri, 2009-05-01 at 03:26 -0400, Jeff Garzik wrote: > Feng Kan wrote: > > Signed-off-by: Feng Kan > > --- > > drivers/ata/Kconfig| 76 +- > > drivers/ata/Makefile |1 + > > drivers/ata/sata_dwc.c | 2047 > > > > 3 files changed, 2091 insertions(+), 33 deletions(-) > > create mode 100644 drivers/ata/sata_dwc.c > > I don't see a patch 2/2 ? Patch 2/2 is just adding the device-tree bits to the platform for this driver to match against, so it's no big deal. However, Feng, we already mentioned that you do a lot of unrelated changes to the Kconfig file that shouldn't be part of this patch, just add the entry for this chip, don't move stuff around in Kconfig, unless I missed a good reason why this is done that way. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2] Add support for Designware SATA controller driver
Feng Kan wrote: Signed-off-by: Feng Kan --- drivers/ata/Kconfig| 76 +- drivers/ata/Makefile |1 + drivers/ata/sata_dwc.c | 2047 3 files changed, 2091 insertions(+), 33 deletions(-) create mode 100644 drivers/ata/sata_dwc.c I don't see a patch 2/2 ? ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2] Add support for Designware SATA controller driver
On Wed, 2009-04-29 at 14:42 -0700, Feng Kan wrote: > Signed-off-by: Feng Kan > --- > drivers/ata/Kconfig| 76 +- > drivers/ata/Makefile |1 + > drivers/ata/sata_dwc.c | 2047 > > 3 files changed, 2091 insertions(+), 33 deletions(-) > create mode 100644 drivers/ata/sata_dwc.c > > diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig > index 0bcf264..5321e47 100644 > --- a/drivers/ata/Kconfig > +++ b/drivers/ata/Kconfig > @@ -72,56 +72,66 @@ config SATA_FSL > > If unsure, say N. > > -config ATA_SFF > - bool "ATA SFF support" > - default y Hi Feng ! Nice to see this driver finally submitted ! However, it should be sent to the linux-ide mailing list in order to be reviewed by the right people (though you should keep linuxppc-dev on CC). Also, I think you want to avoid that Kconfig churn, just add your option as a drop-in, don't re-arrange half of the file while at it :-) Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2] Add support for Designware SATA controller driver
On Wed, 29 Apr 2009 14:42:25 -0700 Feng Kan wrote: > > Signed-off-by: Feng Kan > --- > drivers/ata/Kconfig| 76 +- > drivers/ata/Makefile |1 + > drivers/ata/sata_dwc.c | 2047 > > 3 files changed, 2091 insertions(+), 33 deletions(-) > create mode 100644 drivers/ata/sata_dwc.c > > diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig > index 0bcf264..5321e47 100644 > --- a/drivers/ata/Kconfig > +++ b/drivers/ata/Kconfig > @@ -72,56 +72,66 @@ config SATA_FSL > > If unsure, say N. > > -config ATA_SFF > - bool "ATA SFF support" > - default y > +config SATA_DWC > + tristate "DesignWare Cores SATA support" > + depends on 460EX > help > - This option adds support for ATA controllers with SFF > - compliant or similar programming interface. > + This option enables support for the Synopsys DesignWare Cores SATA > + controller. > + It can be found on the AMCC 460EX. > > - SFF is the legacy IDE interface that has been around since > - the dawn of time. Almost all PATA controllers have an > - SFF interface. Many SATA controllers have an SFF interface > - when configured into a legacy compatibility mode. > + If unsure, say N. > > - For users with exclusively modern controllers like AHCI, > - Silicon Image 3124, or Marvell 6440, you may choose to > - disable this uneeded SFF support. > +config ATA_SFF > +bool "ATA SFF support" > +default y > +help > + This option adds support for ATA controllers with SFF > + compliant or similar programming interface. You seem to have reformatted some of this file (hopefully) unintentionally ... And the linuxppc-embedded mailing list has gone away. -- Cheers, Stephen Rothwells...@canb.auug.org.au http://www.canb.auug.org.au/~sfr/ pgp8MAmR47RT1.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2] Add support for Designware SATA controller driver
Feng, Can you just insert my Signed off by line, using Mark Miesfeld I always goof up the formatting. If that is okay with you and meets your ethical standards. -- Mark Miesfeld miesf...@gmail.com On Wed, Apr 29, 2009 at 4:28 PM, Wolfgang Denk wrote: > Dear Feng Kan, > > In message <1241042419-19774-1-git-send-email-f...@amcc.com> you wrote: >> Signed-off-by: Feng Kan >> --- >> drivers/ata/Kconfig | 76 +- >> drivers/ata/Makefile | 1 + >> drivers/ata/sata_dwc.c | 2047 >> >> 3 files changed, 2091 insertions(+), 33 deletions(-) >> create mode 100644 drivers/ata/sata_dwc.c >> >> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig >> index 0bcf264..5321e47 100644 >> --- a/drivers/ata/Kconfig >> +++ b/drivers/ata/Kconfig >> @@ -72,56 +72,66 @@ config SATA_FSL >> >> If unsure, say N. >> >> -config ATA_SFF >> - bool "ATA SFF support" >> - default y >> +config SATA_DWC >> + tristate "DesignWare Cores SATA support" >> + depends on 460EX >> help >> - This option adds support for ATA controllers with SFF >> - compliant or similar programming interface. >> + This option enables support for the Synopsys DesignWare Cores SATA >> + controller. >> + It can be found on the AMCC 460EX. >> >> - SFF is the legacy IDE interface that has been around since >> - the dawn of time. Almost all PATA controllers have an >> - SFF interface. Many SATA controllers have an SFF interface >> - when configured into a legacy compatibility mode. >> + If unsure, say N. >> >> - For users with exclusively modern controllers like AHCI, >> - Silicon Image 3124, or Marvell 6440, you may choose to >> - disable this uneeded SFF support. >> +config ATA_SFF >> +bool "ATA SFF support" >> +default y >> +help >> + This option adds support for ATA controllers with SFF >> + compliant or similar programming interface. >> >> - If unsure, say Y. >> + SFF is the legacy IDE interface that has been around since >> + the dawn of time. Almost all PATA controllers have an >> + SFF interface. Many SATA controllers have an SFF interface >> + when configured into a legacy compatibility mode. >> + >> + For users with exclusively modern controllers like AHCI, >> + Silicon Image 3124, or Marvell 6440, you may choose to >> + disable this uneeded SFF support. >> + >> + If unsure, say Y. > > Why are you reformatting exiting, correct help text, into brokenness? > > ... >> +static irqreturn_t sata_dwc_isr(int irq, void *dev_instance) >> +{ > ... >> + dev_dbg(ap->dev, "%s non-NCQ cmd interrupt, protocol: %s\n", >> + __func__, prot_2_txt(qc->tf.protocol)); >> +DRVSTILLBUSY: > > ... >> +PROCESS: /* process completed commands */ > ^^^ > > ... >> +STILLBUSY: > ^ > ... >> +DONE: > > > etc. Please do not use all-caps identifier names (not even for > labels). > > ... >> +/*** >> + * Function : sata_dwc_port_start >> + * arguments : struct ata_ioports *port >> + * Return value : returns 0 if success, error code otherwise >> + * This function allocates the scatter gather LLI table for AHB DMA >> + >> **/ > > Here and elsewhere: incorrect multiline comment style. > > ... >> + >> +MODULE_LICENSE("GPL"); >> +MODULE_AUTHOR("Mark Miesfeld "); > > Should not Mark add his Signed-off-by: line, too? > > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de > Conscious is when you are aware of something, and conscience is when > you wish you weren't. > -- > To unsubscribe from this list: send the line "unsubscribe linux-ide" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2] Add support for Designware SATA controller driver
Dear Feng Kan, In message <1241042419-19774-1-git-send-email-f...@amcc.com> you wrote: > Signed-off-by: Feng Kan > --- > drivers/ata/Kconfig| 76 +- > drivers/ata/Makefile |1 + > drivers/ata/sata_dwc.c | 2047 > > 3 files changed, 2091 insertions(+), 33 deletions(-) > create mode 100644 drivers/ata/sata_dwc.c > > diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig > index 0bcf264..5321e47 100644 > --- a/drivers/ata/Kconfig > +++ b/drivers/ata/Kconfig > @@ -72,56 +72,66 @@ config SATA_FSL > > If unsure, say N. > > -config ATA_SFF > - bool "ATA SFF support" > - default y > +config SATA_DWC > + tristate "DesignWare Cores SATA support" > + depends on 460EX > help > - This option adds support for ATA controllers with SFF > - compliant or similar programming interface. > + This option enables support for the Synopsys DesignWare Cores SATA > + controller. > + It can be found on the AMCC 460EX. > > - SFF is the legacy IDE interface that has been around since > - the dawn of time. Almost all PATA controllers have an > - SFF interface. Many SATA controllers have an SFF interface > - when configured into a legacy compatibility mode. > + If unsure, say N. > > - For users with exclusively modern controllers like AHCI, > - Silicon Image 3124, or Marvell 6440, you may choose to > - disable this uneeded SFF support. > +config ATA_SFF > +bool "ATA SFF support" > +default y > +help > + This option adds support for ATA controllers with SFF > + compliant or similar programming interface. > > - If unsure, say Y. > + SFF is the legacy IDE interface that has been around since > + the dawn of time. Almost all PATA controllers have an > + SFF interface. Many SATA controllers have an SFF interface > + when configured into a legacy compatibility mode. > + > + For users with exclusively modern controllers like AHCI, > + Silicon Image 3124, or Marvell 6440, you may choose to > + disable this uneeded SFF support. > + > + If unsure, say Y. Why are you reformatting exiting, correct help text, into brokenness? ... > +static irqreturn_t sata_dwc_isr(int irq, void *dev_instance) > +{ ... > + dev_dbg(ap->dev, "%s non-NCQ cmd interrupt, protocol: %s\n", > + __func__, prot_2_txt(qc->tf.protocol)); > +DRVSTILLBUSY: ... > +PROCESS: /* process completed commands */ ^^^ ... > +STILLBUSY: ^ ... > +DONE: etc. Please do not use all-caps identifier names (not even for labels). ... > +/*** > + * Function : sata_dwc_port_start > + * arguments : struct ata_ioports *port > + * Return value : returns 0 if success, error code otherwise > + * This function allocates the scatter gather LLI table for AHB DMA > + > **/ Here and elsewhere: incorrect multiline comment style. ... > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Mark Miesfeld "); Should not Mark add his Signed-off-by: line, too? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Conscious is when you are aware of something, and conscience is when you wish you weren't. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2] Add support for Designware SATA controller driver
On Apr 29, 2009, at 4:42 PM, Feng Kan wrote: Signed-off-by: Feng Kan --- drivers/ata/Kconfig| 76 +- drivers/ata/Makefile |1 + drivers/ata/sata_dwc.c | 2047 +++ + 3 files changed, 2091 insertions(+), 33 deletions(-) create mode 100644 drivers/ata/sata_dwc.c you should copy Jeff & linux-...@vger.kernel.org on SATA drivers. - k ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev