atang tree: parallel scanning improvements
PATCH 1/1 atang tree: parallel scanning improvements
DISCLAIMER: the fact of getting patches merged into atang tree means that from now on they will be getting updates for changes happening in atang tree and it should not be treated as an indication regarding decisions taken by 'upstream' kernel trees
Allow parallel scan and the like to be set without having to stop using the existing full helper functions (this is based on a upstream proposal patch from Alan Cox) and add parallel scanning feature for older Intel PATA controllers.
[ Patches for other controllers are welcomed and I will also continue
progressively adding parallel scanning while fixing/testing drivers. ]
The following changes since commit e2adfa56bafbafb65cdb6d8efe4cac5ed7582f88:
Bartlomiej Zolnierkiewicz (1):
pata_pdc202xx_old: update documentation
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bart/misc.git atang-v4.6
Bartlomiej Zolnierkiewicz (3):
libata: pass host flags into __ata_pci_sff_init_one() helper
pata_oldpiix: add locking for parallel scanning
pata_oldpiix: enable parallel scan
drivers/ata/libata-sff.c | 4 +++- drivers/ata/pata_ali.c | 2 +- drivers/ata/pata_amd.c | 2 +- drivers/ata/pata_artop.c | 2 +- drivers/ata/pata_atiixp.c | 18 ++---------------- drivers/ata/pata_cmd640.c | 2 +- drivers/ata/pata_cs5530.c | 2 +- drivers/ata/pata_efar.c | 18 ++---------------- drivers/ata/pata_hpt366.c | 2 +- drivers/ata/pata_it821x.c | 2 +- drivers/ata/pata_ns87415.c | 2 +- drivers/ata/pata_oldpiix.c | 13 +++++++++++-- drivers/ata/pata_sl82c105.c | 2 +- include/linux/libata.h | 4 ++-- 14 files changed, 29 insertions(+), 46 deletions(-)
PATCH 1/3 libata: pass host flags into __ata_pci_sff_init_one() helper
From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] libata: pass host flags into __ata_pci_sff_init_one() helper
This was orginally proposed by Alan Cox but as a change for ata_pci_sff_init_one() helper function instead of __ata_pci_sff_init_one() one which casues needless churn to all host drivers and accidentally breakes few host drivers which are still on their way upstream.
Allows parallel scan and the like to be set without having to stop using the existing full helper functions.
--- drivers/ata/libata-sff.c | 4 +++- drivers/ata/pata_ali.c | 2 +- drivers/ata/pata_amd.c | 2 +- drivers/ata/pata_artop.c | 2 +- drivers/ata/pata_atiixp.c | 18 ++---------------- drivers/ata/pata_cmd640.c | 2 +- drivers/ata/pata_cs5530.c | 2 +- drivers/ata/pata_efar.c | 18 ++---------------- drivers/ata/pata_hpt366.c | 2 +- drivers/ata/pata_it821x.c | 2 +- drivers/ata/pata_ns87415.c | 2 +- drivers/ata/pata_sl82c105.c | 2 +- include/linux/libata.h | 4 ++-- 13 files changed, 18 insertions(+), 44 deletions(-)
drivers/ata/libata-sff.c +3 -1
@@ -3009,6 +3009,7 @@ EXPORT_SYMBOL_GPL(ata_pci_sff_activate_h* @sht: scsi_host_template to use when registering the host * @host_priv: host private_data * @init_host: host initialization method (optional) +* @hflag: host flags * * This is a helper function which can be called from a driver's * xxx_init_one() probe function if the hardware uses traditional
@@ -3031,7 +3032,7 @@ EXPORT_SYMBOL_GPL(ata_pci_sff_activate_hint __ata_pci_sff_init_one (struct pci_dev *pdev,const struct ata_port_info *const *ppi,struct scsi_host_template *sht,void *host_priv, -int (*init_host)(struct device *dev)) +int (*init_host)(struct device *dev),int hflag) {struct device *dev = &pdev->dev;const struct ata_port_info *pi =NULL ;
@@ -3068,6 +3069,7 @@ int __ata_pci_sff_init_one(struct pci_de host->private_data = host_priv; host->init_host = init_host; + host->flags |= hflag;pci_set_master (pdev); rc =ata_pci_sff_activate_host (host, ata_sff_interrupt, sht);
drivers/ata/pata_ali.c +1 -1
@@ -316,7 +316,7 @@ static int ali_init_one(struct pci_dev * }return __ata_pci_sff_init_one (pdev, ppi, &ali_sht,NULL , - ali_init_chipset); + ali_init_chipset,0 ); }static const struct pci_device_id ali[] = {
drivers/ata/pata_amd.c +1 -1
@@ -494,7 +494,7 @@ static int amd_init_one(struct pci_dev * }/* And fire it up */ -return __ata_pci_sff_init_one (pdev, ppi, &amd_sht, hpriv, amd_fixup); +return __ata_pci_sff_init_one (pdev, ppi, &amd_sht, hpriv, amd_fixup,0 ); }static const struct pci_device_id amd[] = {
drivers/ata/pata_artop.c +1 -1
@@ -177,7 +177,7 @@ static int artop_init_one (struct pci_deatp8xx_fixup (&pdev->dev);return __ata_pci_sff_init_one (pdev, ppi, &artop_sht,NULL , - atp8xx_fixup); + atp8xx_fixup,0 ); }static const struct pci_device_id artop_pci_tbl[] = {
drivers/ata/pata_atiixp.c +2 -16
@@ -77,23 +77,9 @@ static int atiixp_init_one(struct pci_de .port_ops = &atiixp_port_ops };const struct ata_port_info *ppi[] = { &info, &info }; -struct ata_host *host; -int rc; -/* enable device and prepare host */ - rc =pcim_enable_device (pdev); -if (rc) -return rc; - - rc =ata_pci_sff_prepare_host (pdev, ppi, &host); -if (rc) -return rc; - - host->flags |=ATA_HOST_PARALLEL_SCAN ; - -pci_set_master (pdev); - -return ata_pci_sff_activate_host (host, ata_sff_interrupt, &atiixp_sht); +return __ata_pci_sff_init_one (pdev, ppi, &atiixp_sht,NULL ,NULL , +ATA_HOST_PARALLEL_SCAN ); }static const struct pci_device_id atiixp[] = {
drivers/ata/pata_cmd640.c +1 -1
@@ -226,7 +226,7 @@ static int cmd640_init_one(struct pci_decmd640_hardware_init (&pdev->dev);return __ata_pci_sff_init_one (pdev, ppi, &cmd640_sht,NULL , - cmd640_hardware_init); + cmd640_hardware_init,0 ); }static const struct pci_device_id cmd640[] = {
drivers/ata/pata_cs5530.c +1 -1
@@ -150,7 +150,7 @@ static int cs5530_init_one(struct pci_de/* Now kick off ATA set up */ return __ata_pci_sff_init_one (pdev, ppi, &cs5530_sht,NULL , - cs5530_init_chip); + cs5530_init_chip,0 ); }static const struct pci_device_id cs5530[] = {
drivers/ata/pata_efar.c +2 -16
@@ -89,27 +89,13 @@ static int efar_init_one (struct pci_dev .port_ops = &efar_ops, };const struct ata_port_info *ppi[] = { &info, &info }; -struct ata_host *host; -int rc;if (!printed_version++)dev_printk (KERN_DEBUG , &pdev->dev,"version " DRV_VERSION "\n" ); -/* enable device and prepare host */ - rc =pcim_enable_device (pdev); -if (rc) -return rc; - - rc =ata_pci_sff_prepare_host (pdev, ppi, &host); -if (rc) -return rc; - - host->flags |=ATA_HOST_PARALLEL_SCAN ; - -pci_set_master (pdev); - -return ata_pci_sff_activate_host (host, ata_sff_interrupt, &efar_sht); +return __ata_pci_sff_init_one (pdev, ppi, &efar_sht,NULL ,NULL , +ATA_HOST_PARALLEL_SCAN ); }static const struct pci_device_id efar_pci_tbl[] = {
drivers/ata/pata_hpt366.c +1 -1
@@ -413,7 +413,7 @@ static int hpt36x_init_one(struct pci_de }/* Now kick off ATA set up */ return __ata_pci_sff_init_one (dev, ppi, &hpt36x_sht, hpriv, - hpt36x_init_chipset); + hpt36x_init_chipset,0 ); }static const struct pci_device_id hpt36x[] = {
drivers/ata/pata_it821x.c +1 -1
@@ -625,7 +625,7 @@ static int it821x_init_one(struct pci_de ppi[0 ] = &info_smart; }return __ata_pci_sff_init_one (pdev, ppi, &it821x_sht,NULL , - it821x_fixup); + it821x_fixup,0 ); }static const struct pci_device_id it821x[] = {
drivers/ata/pata_ns87415.c +1 -1
@@ -385,7 +385,7 @@ static int ns87415_init_one (struct pci_ns87415_fixup (&pdev->dev);return __ata_pci_sff_init_one (pdev, ppi, &ns87415_sht,NULL , - ns87415_fixup); + ns87415_fixup,0 ); }static const struct pci_device_id ns87415_pci_tbl[] = {
drivers/ata/pata_sl82c105.c +1 -1
@@ -222,7 +222,7 @@ static int sl82c105_init_one(struct pci_sl82c105_fixup (&dev->dev);return __ata_pci_sff_init_one (dev, ppi, &sl82c105_sht,NULL , - sl82c105_fixup); + sl82c105_fixup,0 ); }static const struct pci_device_id sl82c105[] = {
include/linux/libata.h +2 -2
@@ -1605,12 +1605,12 @@ extern int ata_pci_sff_activate_host(strextern int __ata_pci_sff_init_one (struct pci_dev *pdev,const struct ata_port_info *const *ppi,struct scsi_host_template *sht,void *host_priv, -int (*init_host)(struct device *dev)); +int (*init_host)(struct device *dev),int hflag);static inline int ata_pci_sff_init_one (struct pci_dev *pdev,const struct ata_port_info *const *ppi,struct scsi_host_template *sht,void *host_priv) { -return __ata_pci_sff_init_one (pdev, ppi, sht, host_priv,NULL ); +return __ata_pci_sff_init_one (pdev, ppi, sht, host_priv,NULL ,0 ); }#endif /* CONFIG_PCI */
Alan Cox
NAKRe: [PATCH 1/3] libata: pass host flags into __ata_pci_sff_init_one() helper
Quoted 11 lines · On Thu, 18 Feb 2010 19:59:22 +0100 Bartlomiej Zolnierkiewicz…
From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] libata: pass host flags into__ata_pci_sff_init_one()helperThis was orginally proposed by Alan Cox but as a change for
ata_pci_sff_init_one()helper function instead of__ata_pci_sff_init_one()one which casues needless churn to all host drivers and accidentally breakes few host drivers which are still on their way upstream.Allows parallel scan and the like to be set without having to stop using the existing full helper functions.
NAK - __ is for internal symbol names.
I was split 50/50 on adding ata_pci_sff_init_one_flags() or similar but the churn, given its a one off and we can then add all sorts of other future flags without pain, seemed worth it.
I'm ambivalent about whether its best to go with a new function name as you have or take the hit now (which seems sensible to me). Either way the __ naming is wrong for an external interface.
Anyway I'd *hope* we can get > 50% of interfaces parallel scanning at which point it ceases to be more noise anyway !
Jeff ?
Re: [PATCH 1/3] libata: pass host flags into __ata_pci_sff_init_one() helper
Quoted 12 lines · Alan Cox
Quoted 9 lines · On Thu, 18 Feb 2010 19:59:22 +0100 Bartlomiej Zolnierkiewicz…
From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] libata: pass host flags into__ata_pci_sff_init_one()helperThis was orginally proposed by Alan Cox but as a change for
ata_pci_sff_init_one()helper function instead of__ata_pci_sff_init_one()one which casues needless churn to all host drivers and accidentally breakes few host drivers which are still on their way upstream.
Quoted 4 lines
Quoted 2 lines
Allows parallel scan and the like to be set without having to stop using the existing full helper functions.
NAK - __ is for internal symbol names.
A NAK for an atang specific patch? :)
- Please read DISCLAIMER in the first mail (it is there for a valid reasons).
- The patch is for atang tree and said __ change happened _months_ ago as a part of (very much related) ->init_host work which is not 'upstream'.
- __ has been also used for exactly such purpose in the kernel, i.e.:
$ grep EXPORT_SYMBOL drivers/pci/*c|grep __ drivers/pci/htirq.c:EXPORT_SYMBOL(__ht_create_irq); drivers/pci/pci.c:EXPORT_SYMBOL_GPL(__pci_complete_power_transition); drivers/pci/pci.c:EXPORT_SYMBOL_GPL(__pci_reset_function); drivers/pci/pci-driver.c:EXPORT_SYMBOL(__pci_register_driver);
Just see commit 863b18f introducing __pci_register_driver (please notice that the change was suggested by Al Viro!)..
--
Bartlomiej Zolnierkiewicz
Re: [PATCH 1/3] libata: pass host flags into __ata_pci_sff_init_one() helper
Quoted 1 line
- __ has been also used for exactly such purpose in the kernel, i.e.:
As helpers for inlines and like the yes...
Matter of style anyway.
Re: [PATCH 1/3] libata: pass host flags into __ata_pci_sff_init_one() helper
In reply to Alan CoxQuoted 17 lines · Alan Cox
Quoted 11 lines · On Thu, 18 Feb 2010 19:59:22 +0100 Bartlomiej…
From: Bartlomiej Zolnierkiewicz<[email protected]>
Subject: [PATCH] libata: pass host flags into__ata_pci_sff_init_one()helperThis was orginally proposed by Alan Cox but as a change for
ata_pci_sff_init_one()helper function instead of__ata_pci_sff_init_one()one which casues needless churn to all host drivers and accidentally breakes few host drivers which are still on their way upstream.Allows parallel scan and the like to be set without having to stop using the existing full helper functions.
NAK - __ is for internal symbol names.
Quoted 7 lines
I was split 50/50 on adding
ata_pci_sff_init_one_flags()or similar but the churn, given its a one off and we can then add all sorts of other future flags without pain, seemed worth it.I'm ambivalent about whether its best to go with a new function name as you have or take the hit now (which seems sensible to me). Either way the __ naming is wrong for an external interface.
Your proposed patch from yesterday adding flags to each is fine. All-driver patches are just fine, as long as the need is demonstrated [and it is, in this case].
Quoted 4 lines
Anyway I'd *hope* we can get> 50% of interfaces parallel scanning at which point it ceases to be more noise anyway !
Jeff ?
Most hardware should support parallel scanning, sans caveats like chipsets that snoop SET FEATURES - XFER MODE (that command, and only that command, needs synchronization)
Jeff
PATCH 2/3 pata_oldpiix: add locking for parallel scanning
In reply to Bartlomiej ZolnierkiewiczFrom: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] pata_oldpiix: add locking for parallel scanning
Add an extra locking for parallel scanning.
This is similar change as commit 60c3be3 for ata_piix host driver and while pata_oldpiix doesn't enable parallel scan yet the race could probably also be triggered by requesting re-scanning of both ports at the same time using SCSI sysfs interface.
Fix documentation while at it.
--- drivers/ata/pata_oldpiix.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/ata/pata_oldpiix.c +9 -1
@@ -1,7 +1,8 @@/* -* pata_oldpiix.c - Intel PATA/SATA controllers +* pata_oldpiix.c - older Intel PATA controllers * * (C) 2005 Red Hat +* (C) 2010 Bartlomiej Zolnierkiewicz * * Some parts based on ata_piix.c by Jeff Garzik and others. *
@@ -50,10 +51,13 @@ static int oldpiix_pre_reset(struct ata_return ata_sff_prereset (link, deadline); } +static DEFINE_SPINLOCK (piix_lock); +static void oldpiix_set_timings (struct ata_port *ap,struct ata_device *adev,u8 pio,bool use_mwdma) {struct pci_dev *dev =to_pci_dev (ap->host->dev); +unsigned long flags;unsigned int idetm_port= ap->port_no ?0x42 :0x40 ;u16 idetm_data;int control =0 ;
@@ -84,6 +88,8 @@ static void oldpiix_set_timings(struct a/* Enable DMA timing only */ control |=8 ;/* PIO cycles in PIO0 */ +spin_lock_irqsave (&piix_lock, flags); +pci_read_config_word (dev, idetm_port, &idetm_data);/*
@@ -101,6 +107,8 @@ static void oldpiix_set_timings(struct a (timings[pio][1 ] <<8 );pci_write_config_word (dev, idetm_port, idetm_data); +spin_unlock_irqrestore (&piix_lock, flags); +/* Track which port is configured */ ap->private_data = adev; }
Re: [PATCH 2/3] pata_oldpiix: add locking for parallel scanning
Quoted 5 lines · On Thu, 18 Feb 2010 19:59:28 +0100 Bartlomiej Zolnierkiewicz…
From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH]pata_oldpiix: add locking for parallel scanningAdd an extra locking for parallel scanning.
Quoted 8 lines
This is similar change as commit 60c3be3 for
ata_piixhost driver and whilepata_oldpiixdoesn't enable parallel scan yet the race could probably also be triggered by requesting re-scanning of both ports at the same time using SCSI sysfs interface.Fix documentation while at it.
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
PATCH 3/3 pata_oldpiix: enable parallel scan
In reply to Bartlomiej ZolnierkiewiczFrom: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] pata_oldpiix: enable parallel scan
--- drivers/ata/pata_oldpiix.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/ata/pata_oldpiix.c +2 -1
@@ -215,7 +215,8 @@ static int oldpiix_init_one (struct pci_dev_printk (KERN_DEBUG , &pdev->dev,"version " DRV_VERSION "\n" ); -return ata_pci_sff_init_one (pdev, ppi, &oldpiix_sht,NULL ); +return __ata_pci_sff_init_one (pdev, ppi, &oldpiix_sht,NULL ,NULL , +ATA_HOST_PARALLEL_SCAN ); }static const struct pci_device_id oldpiix_pci_tbl[] = {