[PATCH 2/2] drivers/ata: export some static functions in libata-core.c

[PATCH 2/2] drivers/ata: export some static functions in libata-core.c

[Akira Iguchi]

This patch modifies some static functions in libata-core.c
and libata.h to use them in ata_scc.c.

Signed-off-by: Kou Ishizaki <kou.ishizaki@toshiba.co.jp>
Signed-off-by: Akira Iguchi <akira2.iguchi@toshiba.co.jp>
---

--- linux-2.6.20-rc4/drivers/ata/libata-core.c.orig	2007-01-11 01:56:11.000000000 +0900
+++ linux-2.6.20-rc4/drivers/ata/libata-core.c	2007-01-11 01:56:34.000000000 +0900
@@ -767,7 +767,7 @@ unsigned int ata_dev_classify(const stru
  *	Device type - %ATA_DEV_ATA, %ATA_DEV_ATAPI or %ATA_DEV_NONE.
  */
 
-static unsigned int
+unsigned int
 ata_dev_try_classify(struct ata_port *ap, unsigned int device, u8 *r_err)
 {
 	struct ata_taskfile tf;
@@ -2528,26 +2528,6 @@ int ata_set_mode(struct ata_port *ap, st
 }
 
 /**
- *	ata_tf_to_host - issue ATA taskfile to host controller
- *	@ap: port to which command is being issued
- *	@tf: ATA taskfile register set
- *
- *	Issues ATA taskfile register set to ATA host controller,
- *	with proper synchronization with interrupt handler and
- *	other threads.
- *
- *	LOCKING:
- *	spin_lock_irqsave(host lock)
- */
-
-static inline void ata_tf_to_host(struct ata_port *ap,
-				  const struct ata_taskfile *tf)
-{
-	ap->ops->tf_load(ap, tf);
-	ap->ops->exec_command(ap, tf);
-}
-
-/**
  *	ata_busy_sleep - sleep until BSY clears, or timeout
  *	@ap: port containing status register to be polled
  *	@tmout_pat: impatience timeout
@@ -4080,7 +4060,7 @@ static void ata_pio_sector(struct ata_qu
  *	Inherited from caller.
  */
 
-static void ata_pio_sectors(struct ata_queued_cmd *qc)
+void ata_pio_sectors(struct ata_queued_cmd *qc)
 {
 	if (is_multi_taskfile(&qc->tf)) {
 		/* READ/WRITE MULTIPLE */
@@ -4107,7 +4087,7 @@ static void ata_pio_sectors(struct ata_q
  *	caller.
  */
 
-static void atapi_send_cdb(struct ata_port *ap, struct ata_queued_cmd *qc)
+void atapi_send_cdb(struct ata_port *ap, struct ata_queued_cmd *qc)
 {
 	/* send SCSI cdb */
 	DPRINTK("send cdb\n");
@@ -4236,7 +4216,7 @@ next_sg:
  *	Inherited from caller.
  */
 
-static void atapi_pio_bytes(struct ata_queued_cmd *qc)
+void atapi_pio_bytes(struct ata_queued_cmd *qc)
 {
 	struct ata_port *ap = qc->ap;
 	struct ata_device *dev = qc->dev;
@@ -4277,33 +4257,6 @@ err_out:
 }
 
 /**
- *	ata_hsm_ok_in_wq - Check if the qc can be handled in the workqueue.
- *	@ap: the target ata_port
- *	@qc: qc on going
- *
- *	RETURNS:
- *	1 if ok in workqueue, 0 otherwise.
- */
-
-static inline int ata_hsm_ok_in_wq(struct ata_port *ap, struct ata_queued_cmd *qc)
-{
-	if (qc->tf.flags & ATA_TFLAG_POLLING)
-		return 1;
-
-	if (ap->hsm_task_state == HSM_ST_FIRST) {
-		if (qc->tf.protocol == ATA_PROT_PIO &&
-		    (qc->tf.flags & ATA_TFLAG_WRITE))
-		    return 1;
-
-		if (is_atapi_taskfile(&qc->tf) &&
-		    !(qc->dev->flags & ATA_DFLAG_CDB_INTR))
-			return 1;
-	}
-
-	return 0;
-}
-
-/**
  *	ata_hsm_qc_complete - finish a qc running on standard HSM
  *	@qc: Command to complete
  *	@in_wq: 1 if called from workqueue, 0 otherwise
@@ -6500,3 +6453,10 @@ EXPORT_SYMBOL_GPL(ata_eh_thaw_port);
 EXPORT_SYMBOL_GPL(ata_eh_qc_complete);
 EXPORT_SYMBOL_GPL(ata_eh_qc_retry);
 EXPORT_SYMBOL_GPL(ata_do_eh);
+
+EXPORT_SYMBOL_GPL(ata_dev_try_classify);
+EXPORT_SYMBOL_GPL(ata_pio_sectors);
+EXPORT_SYMBOL_GPL(atapi_send_cdb);
+EXPORT_SYMBOL_GPL(atapi_pio_bytes);
+EXPORT_SYMBOL_GPL(ata_dev_select);
+EXPORT_SYMBOL_GPL(ata_probe_ent_alloc);
--- linux-2.6.20-rc4/include/linux/libata.h.orig	2007-01-11 01:56:22.000000000 +0900
+++ linux-2.6.20-rc4/include/linux/libata.h	2007-01-11 01:56:34.000000000 +0900
@@ -761,6 +761,12 @@ extern void ata_port_queue_task(struct a
 extern u32 ata_wait_register(void __iomem *reg, u32 mask, u32 val,
 			     unsigned long interval_msec,
 			     unsigned long timeout_msec);
+unsigned int ata_dev_try_classify(struct ata_port *ap, unsigned int device, u8 *r_err);
+void ata_pio_sectors(struct ata_queued_cmd *qc);
+void atapi_send_cdb(struct ata_port *ap, struct ata_queued_cmd *qc);
+void atapi_pio_bytes(struct ata_queued_cmd *qc);
+void ata_dev_select(struct ata_port *ap, unsigned int device, unsigned int wait, unsigned int can_sleep);
+struct ata_probe_ent *ata_probe_ent_alloc(struct device *dev, const struct ata_port_info *port);
 
 /*
  * Default driver ops implementations
@@ -1244,4 +1250,51 @@ static inline struct ata_port *ata_shost
 	return (struct ata_port *) &host->hostdata[0];
 }
 
+/**
+ *	ata_tf_to_host - issue ATA taskfile to host controller
+ *	@ap: port to which command is being issued
+ *	@tf: ATA taskfile register set
+ *
+ *	Issues ATA taskfile register set to ATA host controller,
+ *	with proper synchronization with interrupt handler and
+ *	other threads.
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(host lock)
+ */
+
+static inline void ata_tf_to_host(struct ata_port *ap,
+				  const struct ata_taskfile *tf)
+{
+	ap->ops->tf_load(ap, tf);
+	ap->ops->exec_command(ap, tf);
+}
+
+/**
+ *	ata_hsm_ok_in_wq - Check if the qc can be handled in the workqueue.
+ *	@ap: the target ata_port
+ *	@qc: qc on going
+ *
+ *	RETURNS:
+ *	1 if ok in workqueue, 0 otherwise.
+ */
+
+static inline int ata_hsm_ok_in_wq(struct ata_port *ap, struct ata_queued_cmd *qc)
+{
+	if (qc->tf.flags & ATA_TFLAG_POLLING)
+		return 1;
+
+	if (ap->hsm_task_state == HSM_ST_FIRST) {
+		if (qc->tf.protocol == ATA_PROT_PIO &&
+		    (qc->tf.flags & ATA_TFLAG_WRITE))
+		    return 1;
+
+		if (is_atapi_taskfile(&qc->tf) &&
+		    !(qc->dev->flags & ATA_DFLAG_CDB_INTR))
+			return 1;
+	}
+
+	return 0;
+}
+
 #endif /* __LINUX_LIBATA_H__ */

Re: [PATCH 2/2] drivers/ata: export some static functions in libata-core.c

[Arnd Bergmann]

On Thursday 11 January 2007 09:56, Akira Iguchi wrote:
> This patch modifies some static functions in libata-core.c
> and libata.h to use them in ata_scc.c.

Exporting new symbols needs some explanation here about why they
should now be global for ata_scc while the existing drivers
did not need them. Since the patch otherwise only moves stuff
around as needed, I assume it's good.

	Arnd <><

Re: [PATCH 2/2] drivers/ata: export some static functions in libata-core.c

[Akira Iguchi]

Hi, Arnd-san.

Thank you for checking our patches.

>On Thursday 11 January 2007 09:56, Akira Iguchi wrote:
>> This patch modifies some static functions in libata-core.c
>> and libata.h to use them in ata_scc.c.
>
>Exporting new symbols needs some explanation here about why they
>should now be global for ata_scc while the existing drivers
>did not need them. Since the patch otherwise only moves stuff
>around as needed, I assume it's good.

I thought it was better to use common functions as far as possible.
But if it is preferable to copy them in ata_scc.c, I will do so.

Best regards,
Akira Iguchi

Re: [PATCH 2/2] drivers/ata: export some static functions in libata-core.c

[Arnd Bergmann]

On Thursday 11 January 2007 10:30, Akira Iguchi wrote:
> >Exporting new symbols needs some explanation here about why they
> >should now be global for ata_scc while the existing drivers
> >did not need them. Since the patch otherwise only moves stuff
> >around as needed, I assume it's good.
> 
> I thought it was better to use common functions as far as possible.
> But if it is preferable to copy them in ata_scc.c, I will do so.

To clarify my words here:

It is definitely better to use common functions or to modify the common
code rather than copy it.

This particular patch (2/2) only moves code around, so what I meant is
that I'm confident that it does not introduce bugs.

	Arnd <><

Re: [PATCH 2/2] drivers/ata: export some static functions in libata-core.c

[Linas Vepstas]

On Thu, Jan 11, 2007 at 06:30:55PM +0900, Akira Iguchi wrote:
> Hi, Arnd-san.
> 
> Thank you for checking our patches.
> 
> >On Thursday 11 January 2007 09:56, Akira Iguchi wrote:
> >> This patch modifies some static functions in libata-core.c
> >> and libata.h to use them in ata_scc.c.
> >
> >Exporting new symbols needs some explanation here about why they
> >should now be global for ata_scc while the existing drivers
> >did not need them. Since the patch otherwise only moves stuff
> >around as needed, I assume it's good.
> 
> I thought it was better to use common functions as far as possible.

Yes, I agree that it is better to use common functions when possible.

Why is it that other drivers did not need this functions? Did they
copy them?  Or maybe they don't need them?

--linas

Re: [PATCH 2/2] drivers/ata: export some static functions in libata-core.c

[Benjamin Herrenschmidt]

On Thu, 2007-01-11 at 18:30 +0900, Akira Iguchi wrote:
> Hi, Arnd-san.
> 
> Thank you for checking our patches.
> 
> >On Thursday 11 January 2007 09:56, Akira Iguchi wrote:
> >> This patch modifies some static functions in libata-core.c
> >> and libata.h to use them in ata_scc.c.
> >
> >Exporting new symbols needs some explanation here about why they
> >should now be global for ata_scc while the existing drivers
> >did not need them. Since the patch otherwise only moves stuff
> >around as needed, I assume it's good.
> 
> I thought it was better to use common functions as far as possible.
> But if it is preferable to copy them in ata_scc.c, I will do so.

Nah, it's not preferable. I think what is preferable on the contrary is
to make even more common function generic enough so you can use them
instead of copying them.

Ben.

Re: [PATCH 2/2] drivers/ata: export some static functions in libata-core.c

[Akira Iguchi]

Hi, Linas-san, Arnd-san.

>Why is it that other drivers did not need this functions? Did they
>copy them?  Or maybe they don't need them?

As I mentioned in PATCH 1/2, main reason is that this PATA controller
supports only 32bit accesses and that I need to copy all functions
which have low-level access in ata_scc.c.
In these functions, static functions I exported are used.
I thought exporting them was better than copying them.

But other drivers can use functions which have low-level access as it is.
Therefore they don't need to export static functions.

Now, I am trying to implement low-level calls in struct ata_port_operations
to deal with this problem. Please wait a moment.


>-static unsigned int
>+unsigned int
> ata_dev_try_classify(struct ata_port *ap, unsigned int device, u8 *r_err)

I use this in scc_std_softreset(), which is almost same as
ata_std_softreset(). But I cannot use it because ata_devchk() and
ata_bus_softreset() called by it use low-level access.


>-static void ata_pio_sectors(struct ata_queued_cmd *qc)
>+void ata_pio_sectors(struct ata_queued_cmd *qc)
..
>-static void atapi_send_cdb(struct ata_port *ap, struct ata_queued_cmd *qc)
>+void atapi_send_cdb(struct ata_port *ap, struct ata_queued_cmd *qc)
..
>-static void atapi_pio_bytes(struct ata_queued_cmd *qc)
>+void atapi_pio_bytes(struct ata_queued_cmd *qc)

I use them in scc_hsm_move(), which is almost same as ata_hsm_move().
ata_hsm_move() calls ata_hsm_qc_complete() and it calls ata_irq_on().
There is a low-level access in ata_irq_on(), so I cannot use it.


Best regards,
Akira Iguchi

Re: [PATCH 2/2] drivers/ata: export some static functions in libata-core.c

[Benjamin Herrenschmidt]

> Why is it that other drivers did not need this functions? Did they
> copy them?  Or maybe they don't need them?

Most drivers can use the common functions as is. The problem starts when
you need special mecanisms to access the taskfile registers.

Ben.