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
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
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
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
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
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 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
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
