Skip to contentSkip to conversation

Add support for kirkwood sata controller

PATCH New Tor Krill 3 participants 4 messages Jun 24, 2010 – Sep 6, 2010 Block layer

lore
Share & tools
lore

Tor Krill

author
#0 lore raw

PATCH 1/1 Add support for kirkwood sata controller

A rudimentary sata driver for the Marvell Kirkwood CPU based on the architecture of the fsl_sata driver.

Signed-off-by Tor Krill <[email protected]>
---
 drivers/block/Makefile  |    1 +
 drivers/block/sata_mv.c |  800 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/block/sata_mv.h |  229 ++++++++++++++
 3 files changed, 1030 insertions(+), 0 deletions(-)
 create mode 100644 drivers/block/sata_mv.c
 create mode 100644 drivers/block/sata_mv.h
drivers/block/Makefile +1 -0
index 3f6ad5c..5c1de8b 100644
@@ -32,6 +32,7 @@ COBJS-$(CONFIG_IDE_SIL680) += sil680.o
 COBJS-$(CONFIG_LIBATA) += libata.o
 COBJS-$(CONFIG_PATA_BFIN) += pata_bfin.o
 COBJS-$(CONFIG_SATA_DWC) += sata_dwc.o
+COBJS-$(CONFIG_SATA_MV) += sata_mv.o
 COBJS-$(CONFIG_SATA_SIL3114) += sata_sil3114.o
 COBJS-$(CONFIG_SCSI_AHCI) += ahci.o
 COBJS-$(CONFIG_SCSI_SYM53C8XX) += sym53c8xx.o
drivers/block/sata_mv.c new file +800 -0

800 lines not on this page · raw message

drivers/block/sata_mv.h new file +229 -0

229 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#1 lore raw

PATCH 1/1 Add support for kirkwood sata controller

Dear Tor Krill,

Quoted 12 lines · In message <[email protected]> you wrote:

A rudimentary sata driver for the Marvell Kirkwood CPU based on the architecture of the fsl_sata driver.

Signed-off-by Tor Krill <[email protected]>
---
 drivers/block/Makefile  |    1 +
 drivers/block/sata_mv.c |  800 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/block/sata_mv.h |  229 ++++++++++++++
 3 files changed, 1030 insertions(+), 0 deletions(-)
 create mode 100644 drivers/block/sata_mv.c
 create mode 100644 drivers/block/sata_mv.h

...

drivers/block/sata_mv.c line 71

Quoted 11 lines
drivers/block/sata_mv.c new file +11 -0
+#ifdef MALLOC_QUEUE
+	void		*crqb_alloc;
+	struct crqb	*request;
+
+	void		*crpb_alloc;
+	struct crpb	*response;
+
+#else
+	struct crqb request[REQUEST_QUEUE_SIZE] __attribute__ ((aligned(CRQB_ALIGN)));
+	struct crpb response[REQUEST_QUEUE_SIZE] __attribute__ ((aligned(CRPB_ALIGN)));
+#endif

What about always using pointers for request and response, and initializing these statically (taking the address of a diffrently named static array) in one case and dynamically (from malloc() in the
other case? Then we would need the #ifdef's only in one or two
places, and avoid constructs like these:

Quoted 15 lines
drivers/block/sata_mv.c new file +15 -0
+#ifdef MALLOC_QUEUE
+	out_le32(priv->regbase+EDMA_RQIPR, priv->request);
+#else
+	out_le32(priv->regbase+EDMA_RQIPR, &priv->request);
+#endif
+	out_le32(priv->regbase+EDMA_RQOPR, 0x0);
+
+	/* Setup response queue */
+	out_le32(priv->regbase+EDMA_RSBA_HI, 0x0);
+#ifdef MALLOC_QUEUE
+	out_le32(priv->regbase+EDMA_RSOPR, priv->response);
+#else
+	out_le32(priv->regbase+EDMA_RSOPR, &priv->response);
+#endif
+	out_le32(priv->regbase+EDMA_RSIPR, 0x0);

...

drivers/block/sata_mv.c line 227

Quoted 3 lines
drivers/block/sata_mv.c new file +3 -0
+	debug("Probe port: %d\n\r",port);
+
+	for ( tries=0; tries<2; tries++){

No space after the '('.

drivers/block/sata_mv.c line 241

Quoted 3 lines
drivers/block/sata_mv.c new file +3 -0
+		tmp = in_le32(priv->regbase + SIR_SCONTROL);
+		tries2 = 5;
+		do{

Space: "do {"

drivers/block/sata_mv.c line 265

Quoted 1 line
drivers/block/sata_mv.c new file +1 -0
+		if ( !set15 ){

Make this:

	if (!set15) {

drivers/block/sata_mv.c line 292

Quoted 4 lines
drivers/block/sata_mv.c new file +4 -0
+	tmp = in_le32(priv->regbase+EDMA_RQIPR) & EDMA_RQIPR_IPMASK;
+	tmp = tmp >> EDMA_RQIPR_IPSHIFT;
+
+	return tmp;

combine the last 2 lines into

	return tmp >> EDMA_RQIPR_IPSHIFT;

drivers/block/sata_mv.c line 318

Quoted 4 lines
drivers/block/sata_mv.c new file +4 -0
+	tmp = in_le32(priv->regbase+EDMA_RQOPR) & EDMA_RQOPR_OPMASK;
+	tmp = tmp >> EDMA_RQOPR_OPSHIFT;
+
+	return tmp;

Ditto, and so on.

Quoted 8 lines
drivers/block/sata_mv.c new file +8 -0
+
+/* Get response queue in pointer */
+static int get_rspip(int port){
+	u32 tmp;
+	struct mv_priv *priv = (struct mv_priv *) sata_dev_desc[port].priv;
+
+	tmp = in_le32(priv->regbase+EDMA_RSIPR) & EDMA_RSIPR_IPMASK;
+	tmp = tmp >> EDMA_RSIPR_IPSHIFT;

drivers/block/sata_mv.c line 364

Quoted 10 lines
drivers/block/sata_mv.c new file +10 -0
+
+	return tmp;
+}
+
+/* Get response queue out pointer */
+static int get_rspop(int port){
+	u32 tmp;
+	struct mv_priv *priv = (struct mv_priv *) sata_dev_desc[port].priv;
+	if ( (res = ata_wait_register(
+			(u32*)(KW_SATAHC_BASE+SATAHC_ICR), tmp ,tmp, timeout_msec))){

Line too long, spaces after ',' (not before), space before '{'.

drivers/block/sata_mv.c line 365

Quoted 1 line
drivers/block/sata_mv.c new file +1 -0
+		printf("Failed to wait for completion on port %d\n",port);

Space after comma.

drivers/block/sata_mv.c line 382

Quoted 5 lines
drivers/block/sata_mv.c new file +5 -0
+	if ( port == 0 ){
+		tmp &= ~( 1<<0 | 1<<8);
+	}else{
+		tmp &= ~(1<<1 | 1<<9);
+	}

No braces needed for one line statements. If you use braches, they are always separated by a space.

drivers/block/sata_mv.c line 387

Quoted 2 lines
drivers/block/sata_mv.c new file +2 -0
+	while(get_rspip(port)!=outind){
+		debug("Response index %d flags %08x on port %d\n",outind,priv->response[outind].flags,port);

Line too long. Please fix globally.

drivers/block/sata_mv.c line 394

Quoted 2 lines
drivers/block/sata_mv.c new file +2 -0
+static int mv_ata_exec_ata_cmd(int port, struct sata_fis_h2d *cfis,
+				u8 *buffer, u32 len, u32 iswrite){

Brace goes on next line.

drivers/block/sata_mv.c line 457

Quoted 3 lines
drivers/block/sata_mv.c new file +3 -0
+	priv->request[slot].ata_sect_count |=
+			((cfis->sector_count_exp<<CRQB_SECTCOUNT_COUNT_EXP_SHIFT) &
+					CRQB_SECTCOUNT_COUNT_EXP_MASK);

Line too long.

drivers/block/sata_mv.c line 464

Quoted 2 lines
drivers/block/sata_mv.c new file +2 -0
+	/* Wait for completion */
+	if( wait_dma_completion(port,slot,10000) ){

	if (wait_dma_completion(port,slot,10000)) {

Please fix brace style globally.

drivers/block/sata_mv.c line 631

Quoted 2 lines
drivers/block/sata_mv.c new file +2 -0
+	debug("pio %04x, mwdma %04x, udma %04x\n\r"
+			,priv->pio, priv->mwdma, priv->udma);

Comma goes at end of first line.

drivers/block/sata_mv.c line 656

Quoted 11 lines
drivers/block/sata_mv.c new file +11 -0
+	/* First check the device capablity */
+	udma_cap = (u8)(priv->udma & 0xff);
+
+	if (udma_cap == ATA_UDMA6)
+		cfis.sector_count = XFER_UDMA_6;
+	if (udma_cap == ATA_UDMA5)
+		cfis.sector_count = XFER_UDMA_5;
+	if (udma_cap == ATA_UDMA4)
+		cfis.sector_count = XFER_UDMA_4;
+	if (udma_cap == ATA_UDMA3)
+		cfis.sector_count = XFER_UDMA_3;

Use a switch() ?

Handle "default:" case?

drivers/block/sata_mv.c line 679

Quoted 1 line
drivers/block/sata_mv.c new file +1 -0
+	if ( dev < 0 || dev >= CONFIG_SYS_SATA_MAX_DEVICE) {

... ----^

drivers/block/sata_mv.c line 685

Quoted 1 line
drivers/block/sata_mv.c new file +1 -0
+	if ( !priv ){

... ----^-----^-^

drivers/block/sata_mv.c line 690

Quoted 1 line
drivers/block/sata_mv.c new file +1 -0
+	memset((void*) priv,0 , sizeof(struct mv_priv));

... --------------------^^

There are tons of such coding style issues. Please fix globally.

Best regards,

Wolfgang Denk

Signature · 6 lines
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: wd at denx.de
HEALTH WARNING: Care Should Be Taken When Lifting This Product, Since
Its Mass, and Thus Its Weight, Is Dependent on Its Velocity  Relative
to the User.
#2 lore raw

PATCH 1/1 Add support for kirkwood sata controller

Hi Tor

FYI: alternate patch series
http://lists.denx.de/pipermail/u-boot/2010-August/074966.html
On the top of
http://lists.denx.de/pipermail/u-boot/2010-August/074950.html
Will enable this

I hope it will help you

Regards..
Prafulla . .

Quoted 25 lines

-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Wolfgang Denk
Sent: Monday, August 09, 2010 1:44 AM
To: Tor Krill
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH] Add support for kirkwood sata controller

Dear Tor Krill,

In message <[email protected]>

Quoted 8 lines · you

A rudimentary sata driver for the Marvell Kirkwood CPU based on the architecture of the fsl_sata driver.

Signed-off-by Tor Krill <[email protected]>
---
 drivers/block/Makefile  |    1 +
 drivers/block/sata_mv.c |  800

+++++++++++++++++++++++++++++++++++++++++++++++

Quoted 4 lines
 drivers/block/sata_mv.h |  229 ++++++++++++++
 3 files changed, 1030 insertions(+), 0 deletions(-)
 create mode 100644 drivers/block/sata_mv.c
 create mode 100644 drivers/block/sata_mv.h
author
#3 lore raw

PATCH 1/1 Add support for kirkwood sata controller

Quoted 1 line · Prafulla Wadaskar

Hi Tor

Hi Prafulla, Wolfgang and others,

Sorry for the, very, late reply.

Quoted 7 lines

FYI: alternate patch series
http://lists.denx.de/pipermail/u-boot/2010-August/074966.html
On the top of
http://lists.denx.de/pipermail/u-boot/2010-August/074950.html
Will enable this

I hope it will help you

Thanks for the pointers, and thanks Wolfgang for reviewing the driver.

I assume that Prafulla has made an objective review of the alternatives and chosen the ata/taskfile compatibility version of the driver.

Since my requirements of the driver is somewhat different i think i just keep my code out of the tree for now.

Best,

/Tor