Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH 06/10] drivers/scsi/qla2xxx/qua_os.c: Fix continuation line formats

4 views
Skip to first unread message

Joe Perches

unread,
Feb 2, 2010, 2:30:02 AM2/2/10
to
String constants that are continued on subsequent lines with \
are not good.

Signed-off-by: Joe Perches <j...@perches.com>
---
drivers/scsi/qla2xxx/qla_os.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 8529eb1..134ed38 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -88,8 +88,8 @@ int ql2xmaxqueues = 1;
module_param(ql2xmaxqueues, int, S_IRUGO|S_IRUSR);
MODULE_PARM_DESC(ql2xmaxqueues,
"Enables MQ settings "
- "Default is 1 for single queue. Set it to number \
- of queues in MQ mode.");
+ "Default is 1 for single queue. Set it to number "
+ "of queues in MQ mode.");

int ql2xmultique_tag;
module_param(ql2xmultique_tag, int, S_IRUGO|S_IRUSR);
--
1.6.6.rc0.57.gad7a

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Joe Perches

unread,
Feb 2, 2010, 2:30:02 AM2/2/10
to
String constants that are continued on subsequent lines with \
are not good.

Signed-off-by: Joe Perches <j...@perches.com>
---

drivers/net/davinci_emac.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
index 33c4fe2..faffad4 100644
--- a/drivers/net/davinci_emac.c
+++ b/drivers/net/davinci_emac.c
@@ -2672,8 +2672,7 @@ static int __devinit davinci_emac_probe(struct platform_device *pdev)
priv->emac_base_phys = res->start + pdata->ctrl_reg_offset;
size = res->end - res->start + 1;
if (!request_mem_region(res->start, size, ndev->name)) {
- dev_err(emac_dev, "DaVinci EMAC: failed request_mem_region() \
- for regs\n");
+ dev_err(emac_dev, "DaVinci EMAC: failed request_mem_region() for regs\n");
rc = -ENXIO;
goto probe_quit;

Joe Perches

unread,
Feb 2, 2010, 2:30:03 AM2/2/10
to
String constants that are continued on subsequent lines with \
are not good.

Signed-off-by: Joe Perches <j...@perches.com>
---

arch/ia64/kvm/mmio.c | 4 ++--
arch/ia64/kvm/vcpu.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/ia64/kvm/mmio.c b/arch/ia64/kvm/mmio.c
index 9bf55af..fb8f9f5 100644
--- a/arch/ia64/kvm/mmio.c
+++ b/arch/ia64/kvm/mmio.c
@@ -316,8 +316,8 @@ void emulate_io_inst(struct kvm_vcpu *vcpu, u64 padr, u64 ma)
return;
} else {
inst_type = -1;
- panic_vm(vcpu, "Unsupported MMIO access instruction! \
- Bunld[0]=0x%lx, Bundle[1]=0x%lx\n",
+ panic_vm(vcpu, "Unsupported MMIO access instruction! "
+ "Bunld[0]=0x%lx, Bundle[1]=0x%lx\n",
bundle.i64[0], bundle.i64[1]);
}

diff --git a/arch/ia64/kvm/vcpu.c b/arch/ia64/kvm/vcpu.c
index dce75b7..958815c 100644
--- a/arch/ia64/kvm/vcpu.c
+++ b/arch/ia64/kvm/vcpu.c
@@ -1639,8 +1639,8 @@ void vcpu_set_psr(struct kvm_vcpu *vcpu, unsigned long val)
* Otherwise panic
*/
if (val & (IA64_PSR_PK | IA64_PSR_IS | IA64_PSR_VM))
- panic_vm(vcpu, "Only support guests with vpsr.pk =0 \
- & vpsr.is=0\n");
+ panic_vm(vcpu, "Only support guests with vpsr.pk =0 "
+ "& vpsr.is=0\n");

/*
* For those IA64_PSR bits: id/da/dd/ss/ed/ia

Joe Perches

unread,
Feb 2, 2010, 2:30:02 AM2/2/10
to
String constants that are continued on subsequent lines with \
are not good.

Signed-off-by: Joe Perches <j...@perches.com>
---

sound/soc/s3c24xx/s3c-pcm.c | 3 +--


1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/sound/soc/s3c24xx/s3c-pcm.c b/sound/soc/s3c24xx/s3c-pcm.c
index 9e61a7c..a98f40c 100644
--- a/sound/soc/s3c24xx/s3c-pcm.c
+++ b/sound/soc/s3c24xx/s3c-pcm.c
@@ -229,8 +229,7 @@ static int s3c_pcm_hw_params(struct snd_pcm_substream *substream,

spin_unlock_irqrestore(&pcm->lock, flags);

- dev_dbg(pcm->dev, "PCMSOURCE_CLK-%lu SCLK=%ufs \
- SCLK_DIV=%d SYNC_DIV=%d\n",
+ dev_dbg(pcm->dev, "PCMSOURCE_CLK-%lu SCLK=%ufs SCLK_DIV=%d SYNC_DIV=%d\n",
clk_get_rate(clk), pcm->sclk_per_fs,
sclk_div, sync_div);

Joe Perches

unread,
Feb 2, 2010, 2:30:02 AM2/2/10
to
String constants that are continued on subsequent lines with \
are not good.

Signed-off-by: Joe Perches <j...@perches.com>
---

drivers/staging/rtl8192su/r8192S_phy.c | 16 ++++++++--------
drivers/staging/sep/sep_driver.c | 5 +++--
drivers/staging/slicoss/slicoss.c | 6 ++----
3 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/rtl8192su/r8192S_phy.c b/drivers/staging/rtl8192su/r8192S_phy.c
index 77ab026..63d4e5f 100644
--- a/drivers/staging/rtl8192su/r8192S_phy.c
+++ b/drivers/staging/rtl8192su/r8192S_phy.c
@@ -2407,8 +2407,8 @@ void PHY_SetBWModeCallback8192S(struct net_device *dev)
break;

default:
- RT_TRACE(COMP_DBG, "SetBWModeCallback8190Pci():\
- unknown Bandwidth: %#X\n",priv->CurrentChannelBW);
+ RT_TRACE(COMP_DBG, "SetBWModeCallback8190Pci(): unknown Bandwidth: %#X\n",
+ priv->CurrentChannelBW);
break;
}

@@ -3398,8 +3398,8 @@ void SwChnlCallback8192SUsb(struct net_device *dev)
u32 delay;
// bool ret;

- RT_TRACE(COMP_SCAN, "==>SwChnlCallback8190Pci(), switch to channel\
- %d\n", priv->chan);
+ RT_TRACE(COMP_SCAN, "==>SwChnlCallback8190Pci(), switch to channel %d\n",
+ priv->chan);


if(!priv->up)
@@ -3525,8 +3525,8 @@ void SetBWModeCallback8192SUsb(struct net_device *dev)
break;

default:
- RT_TRACE(COMP_DBG, "SetChannelBandwidth8190Pci():\
- unknown Bandwidth: %#X\n",priv->CurrentChannelBW);
+ RT_TRACE(COMP_DBG, "SetChannelBandwidth8190Pci(): unknown Bandwidth: %#X\n",
+ priv->CurrentChannelBW);
break;
}

@@ -3660,8 +3660,8 @@ void SetBWModeCallback8192SUsbWorkItem(struct net_device *dev)
break;

default:
- RT_TRACE(COMP_DBG, "SetBWModeCallback8192SUsbWorkItem():\
- unknown Bandwidth: %#X\n",priv->CurrentChannelBW);
+ RT_TRACE(COMP_DBG, "SetBWModeCallback8192SUsbWorkItem(): unknown Bandwidth: %#X\n",
+ priv->CurrentChannelBW);
break;
}

diff --git a/drivers/staging/sep/sep_driver.c b/drivers/staging/sep/sep_driver.c
index 8793878..54234f1 100644
--- a/drivers/staging/sep/sep_driver.c
+++ b/drivers/staging/sep/sep_driver.c
@@ -940,8 +940,9 @@ static int sep_lock_user_pages(struct sep_device *sep,
dbg("data_size is %lu\n", data_size);
while (1);
}
- edbg("lli_array[%lu].physical_address is %08lx, \
- lli_array[%lu].block_size is %lu\n", count, lli_array[count].physical_address, count, lli_array[count].block_size);
+ edbg("lli_array[%lu].physical_address is %08lx, lli_array[%lu].block_size is %lu\n",
+ count, lli_array[count].physical_address,
+ count, lli_array[count].block_size);
}

/* set output params */
diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
index 5b191af..4e5158a 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -649,8 +649,7 @@ static int slic_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)

if (copy_from_user(data, rq->ifr_data, 28)) {
PRINT_ERROR
- ("slic: copy_from_user FAILED getting \
- initial simba param\n");
+ ("slic: copy_from_user FAILED getting initial simba param\n");
return -EFAULT;
}

@@ -665,8 +664,7 @@ static int slic_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
(tracemon_request ==
SLIC_DUMP_IN_PROGRESS)) {
PRINT_ERROR
- ("ATK Diagnostic Trace Dump Requested but \
- already in progress... ignore\n");
+ ("ATK Diagnostic Trace Dump Requested but already in progress... ignore\n");
} else {
PRINT_ERROR
("ATK Diagnostic Trace Dump Requested\n");

Joe Perches

unread,
Feb 2, 2010, 2:30:02 AM2/2/10
to
String constants that are continued on subsequent lines with \
are not good.

Signed-off-by: Joe Perches <j...@perches.com>
---

drivers/hwmon/vt8231.c | 3 +--


1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/vt8231.c b/drivers/hwmon/vt8231.c
index d47b4c9..e6078c9 100644
--- a/drivers/hwmon/vt8231.c
+++ b/drivers/hwmon/vt8231.c
@@ -948,8 +948,7 @@ static int __devinit vt8231_pci_probe(struct pci_dev *dev,

address = val & ~(VT8231_EXTENT - 1);
if (address == 0) {
- dev_err(&dev->dev, "base address not set -\
- upgrade BIOS or use force_addr=0xaddr\n");
+ dev_err(&dev->dev, "base address not set - upgrade BIOS or use force_addr=0xaddr\n");
return -ENODEV;

Joe Perches

unread,
Feb 2, 2010, 2:30:03 AM2/2/10
to
String constants that are continued on subsequent lines with \
are not good.

Signed-off-by: Joe Perches <j...@perches.com>
---

drivers/ata/pata_at91.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/pata_at91.c b/drivers/ata/pata_at91.c
index 41c94b1..d222c0d 100644
--- a/drivers/ata/pata_at91.c
+++ b/drivers/ata/pata_at91.c
@@ -153,8 +153,8 @@ static void pata_at91_set_piomode(struct ata_port *ap, struct ata_device *adev)
/* Compute ATA timing and set it to SMC */
ret = ata_timing_compute(adev, adev->pio_mode, &timing, 1000, 0);
if (ret) {
- dev_warn(ap->dev, "Failed to compute ATA timing %d, \
- set PIO_0 timing\n", ret);
+ dev_warn(ap->dev, "Failed to compute ATA timing %d, set PIO_0 timing\n",
+ ret);
set_smc_timing(ap->dev, info, &initial_timing);
} else {
set_smc_timing(ap->dev, info, &timing);

Joe Perches

unread,
Feb 2, 2010, 2:30:03 AM2/2/10
to
String constants that are continued on subsequent lines with \
are not good.

Signed-off-by: Joe Perches <j...@perches.com>
---

drivers/gpu/drm/i915/intel_bios.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 15fbc1b..fe14cde 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -416,8 +416,7 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
edp = find_section(bdb, BDB_EDP);
if (!edp) {
if (SUPPORTS_EDP(dev_priv->dev) && dev_priv->edp_support) {
- DRM_DEBUG_KMS("No eDP BDB found but eDP panel supported,\
- assume 18bpp panel color depth.\n");
+ DRM_DEBUG_KMS("No eDP BDB found but eDP panel supported, assume 18bpp panel color depth.\n");
dev_priv->edp_bpp = 18;
}
return;

Joe Perches

unread,
Feb 2, 2010, 2:30:03 AM2/2/10
to
String constants that are continued on subsequent lines with \
are not good.

Signed-off-by: Joe Perches <j...@perches.com>
---

drivers/video/via/dvi.c | 8 ++++----
drivers/video/via/lcd.c | 12 ++++++------
2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/video/via/dvi.c b/drivers/video/via/dvi.c
index 67b3693..b3b7822 100644
--- a/drivers/video/via/dvi.c
+++ b/drivers/video/via/dvi.c
@@ -467,8 +467,8 @@ static int dvi_get_panel_size_from_DDCv1(void)
default:
viaparinfo->tmds_setting_info->dvi_panel_size =
VIA_RES_1024X768;
- DEBUG_MSG(KERN_INFO "Unknown panel size max resolution = %d !\
- set default panel size.\n", max_h);
+ DEBUG_MSG(KERN_INFO "Unknown panel size max resolution = %d ! "
+ "set default panel size.\n", max_h);
break;
}

@@ -534,8 +534,8 @@ static int dvi_get_panel_size_from_DDCv2(void)
default:
viaparinfo->tmds_setting_info->dvi_panel_size =
VIA_RES_1024X768;
- DEBUG_MSG(KERN_INFO "Unknown panel size max resolution = %d!\
- set default panel size.\n", HSize);
+ DEBUG_MSG(KERN_INFO "Unknown panel size max resolution = %d! "
+ "set default panel size.\n", HSize);
break;
}

diff --git a/drivers/video/via/lcd.c b/drivers/video/via/lcd.c
index 09353e2..4962297 100644
--- a/drivers/video/via/lcd.c
+++ b/drivers/video/via/lcd.c
@@ -171,13 +171,13 @@ static bool lvds_identify_integratedlvds(void)
if (viaparinfo->chip_info->lvds_chip_info.lvds_chip_name) {
viaparinfo->chip_info->lvds_chip_info2.lvds_chip_name =
INTEGRATED_LVDS;
- DEBUG_MSG(KERN_INFO "Support two dual channel LVDS!\
- (Internal LVDS + External LVDS)\n");
+ DEBUG_MSG(KERN_INFO "Support two dual channel LVDS! "
+ "(Internal LVDS + External LVDS)\n");
} else {
viaparinfo->chip_info->lvds_chip_info.lvds_chip_name =
INTEGRATED_LVDS;
- DEBUG_MSG(KERN_INFO "Not found external LVDS,\
- so can't support two dual channel LVDS!\n");
+ DEBUG_MSG(KERN_INFO "Not found external LVDS, "
+ "so can't support two dual channel LVDS!\n");
}
} else if (viafb_display_hardware_layout == HW_LAYOUT_LCD1_LCD2) {
/* Two single channel LCD (Internal LVDS + Internal LVDS): */
@@ -185,8 +185,8 @@ static bool lvds_identify_integratedlvds(void)
INTEGRATED_LVDS;
viaparinfo->chip_info->lvds_chip_info2.lvds_chip_name =
INTEGRATED_LVDS;
- DEBUG_MSG(KERN_INFO "Support two single channel LVDS!\
- (Internal LVDS + Internal LVDS)\n");
+ DEBUG_MSG(KERN_INFO "Support two single channel LVDS! "
+ "(Internal LVDS + Internal LVDS)\n");
} else if (viafb_display_hardware_layout != HW_LAYOUT_DVI_ONLY) {
/* If we have found external LVDS, just use it,
otherwise, we will use internal LVDS as default. */

Joe Perches

unread,
Feb 2, 2010, 2:30:03 AM2/2/10
to
String constants that are continued on subsequent lines with \
are not good.

Signed-off-by: Joe Perches <j...@perches.com>
---

drivers/spi/spi_s3c64xx.c | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c
index 88a456d..0d4e37a 100644
--- a/drivers/spi/spi_s3c64xx.c
+++ b/drivers/spi/spi_s3c64xx.c
@@ -632,8 +632,8 @@ static void handle_msg(struct s3c64xx_spi_driver_data *sdd,
S3C64XX_SPI_DEACT(sdd);

if (status) {
- dev_err(&spi->dev, "I/O Error: \
- rx-%d tx-%d res:rx-%c tx-%c len-%d\n",
+ dev_err(&spi->dev, "I/O Error: "
+ "rx-%d tx-%d res:rx-%c tx-%c len-%d\n",
xfer->rx_buf ? 1 : 0, xfer->tx_buf ? 1 : 0,
(sdd->state & RXBUSY) ? 'f' : 'p',
(sdd->state & TXBUSY) ? 'f' : 'p',
@@ -1040,11 +1040,10 @@ static int __init s3c64xx_spi_probe(struct platform_device *pdev)
goto err8;
}

- dev_dbg(&pdev->dev, "Samsung SoC SPI Driver loaded for Bus SPI-%d \
- with %d Slaves attached\n",
+ dev_dbg(&pdev->dev, "Samsung SoC SPI Driver loaded for Bus SPI-%d "
+ "with %d Slaves attached\n",
pdev->id, master->num_chipselect);
- dev_dbg(&pdev->dev, "\tIOmem=[0x%x-0x%x]\
- \tDMA=[Rx-%d, Tx-%d]\n",
+ dev_dbg(&pdev->dev, "\tIOmem=[0x%x-0x%x]\tDMA=[Rx-%d, Tx-%d]\n",
mem_res->end, mem_res->start,
sdd->rx_dmach, sdd->tx_dmach);

Jean Delvare

unread,
Feb 2, 2010, 3:00:04 AM2/2/10
to
Hi Joe,

On Mon, 1 Feb 2010 23:22:10 -0800, Joe Perches wrote:
> String constants that are continued on subsequent lines with \
> are not good.
>
> Signed-off-by: Joe Perches <j...@perches.com>
> ---
> drivers/hwmon/vt8231.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwmon/vt8231.c b/drivers/hwmon/vt8231.c
> index d47b4c9..e6078c9 100644
> --- a/drivers/hwmon/vt8231.c
> +++ b/drivers/hwmon/vt8231.c
> @@ -948,8 +948,7 @@ static int __devinit vt8231_pci_probe(struct pci_dev *dev,
>
> address = val & ~(VT8231_EXTENT - 1);
> if (address == 0) {
> - dev_err(&dev->dev, "base address not set -\
> - upgrade BIOS or use force_addr=0xaddr\n");
> + dev_err(&dev->dev, "base address not set - upgrade BIOS or use force_addr=0xaddr\n");
> return -ENODEV;
> }
>

Good catch, and thanks for the fix. But now we have an overlong line,
which checkpatch will complain about. So I would prefer if you'd fix
the problem by replacing the \ by a pair of ".

--
Jean Delvare

Joe Perches

unread,
Feb 2, 2010, 3:10:02 AM2/2/10
to
On Tue, 2010-02-02 at 08:56 +0100, Jean Delvare wrote:
> > diff --git a/drivers/hwmon/vt8231.c b/drivers/hwmon/vt8231.c
> > index d47b4c9..e6078c9 100644
> > --- a/drivers/hwmon/vt8231.c
> > +++ b/drivers/hwmon/vt8231.c
> > @@ -948,8 +948,7 @@ static int __devinit vt8231_pci_probe(struct pci_dev *dev,
> >
> > address = val & ~(VT8231_EXTENT - 1);
> > if (address == 0) {
> > - dev_err(&dev->dev, "base address not set -\
> > - upgrade BIOS or use force_addr=0xaddr\n");
> > + dev_err(&dev->dev, "base address not set - upgrade BIOS or use force_addr=0xaddr\n");
> > return -ENODEV;
> > }
> >
>
> Good catch, and thanks for the fix. But now we have an overlong line,
> which checkpatch will complain about. So I would prefer if you'd fix
> the problem by replacing the \ by a pair of ".

That depends on what version of checkpatch you're using.

There's one with a pr_<level> and dev_<level> exemption
to the long line check.

Grant Likely

unread,
Feb 2, 2010, 3:30:01 AM2/2/10
to
On Tue, Feb 2, 2010 at 12:22 AM, Joe Perches <j...@perches.com> wrote:
> String constants that are continued on subsequent lines with \
> are not good.
>
> Signed-off-by: Joe Perches <j...@perches.com>

Merged into next-spi, thanks.

g.

Mark Brown

unread,
Feb 2, 2010, 6:50:01 AM2/2/10
to
On Mon, Feb 01, 2010 at 11:22:16PM -0800, Joe Perches wrote:
> String constants that are continued on subsequent lines with \
> are not good.

Applied, thanks.

Mark Lord

unread,
Feb 3, 2010, 12:30:02 PM2/3/10
to
Joe Perches wrote:
> String constants that are continued on subsequent lines with \
> are not good.
>
> Signed-off-by: Joe Perches <j...@perches.com>
> ---
> drivers/ata/pata_at91.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/ata/pata_at91.c b/drivers/ata/pata_at91.c
> index 41c94b1..d222c0d 100644
> --- a/drivers/ata/pata_at91.c
> +++ b/drivers/ata/pata_at91.c
> @@ -153,8 +153,8 @@ static void pata_at91_set_piomode(struct ata_port *ap, struct ata_device *adev)
> /* Compute ATA timing and set it to SMC */
> ret = ata_timing_compute(adev, adev->pio_mode, &timing, 1000, 0);
> if (ret) {
> - dev_warn(ap->dev, "Failed to compute ATA timing %d, \
> - set PIO_0 timing\n", ret);
> + dev_warn(ap->dev, "Failed to compute ATA timing %d, set PIO_0 timing\n",
> + ret);
..

NAK. Just stick the ret); part onto the same line and be done with it.

-ml

Jeff Garzik

unread,
Feb 3, 2010, 12:30:02 PM2/3/10
to
On 02/03/2010 12:21 PM, Mark Lord wrote:
> Joe Perches wrote:
>> String constants that are continued on subsequent lines with \
>> are not good.
>>
>> Signed-off-by: Joe Perches <j...@perches.com>
>> ---
>> drivers/ata/pata_at91.c | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/ata/pata_at91.c b/drivers/ata/pata_at91.c
>> index 41c94b1..d222c0d 100644
>> --- a/drivers/ata/pata_at91.c
>> +++ b/drivers/ata/pata_at91.c
>> @@ -153,8 +153,8 @@ static void pata_at91_set_piomode(struct ata_port
>> *ap, struct ata_device *adev)
>> /* Compute ATA timing and set it to SMC */
>> ret = ata_timing_compute(adev, adev->pio_mode, &timing, 1000, 0);
>> if (ret) {
>> - dev_warn(ap->dev, "Failed to compute ATA timing %d, \
>> - set PIO_0 timing\n", ret);
>> + dev_warn(ap->dev, "Failed to compute ATA timing %d, set PIO_0
>> timing\n",
>> + ret);
> ..
>
> NAK. Just stick the ret); part onto the same line and be done with it.

Agreed...

Joe Perches

unread,
Feb 3, 2010, 2:20:02 PM2/3/10
to
On Wed, 2010-02-03 at 12:21 -0500, Mark Lord wrote:
> Joe Perches wrote:
> > String constants that are continued on subsequent lines with \
> > are not good.
[]

> > - dev_warn(ap->dev, "Failed to compute ATA timing %d, \
> > - set PIO_0 timing\n", ret);
> > + dev_warn(ap->dev, "Failed to compute ATA timing %d, set PIO_0 timing\n",
> > + ret);
> NAK. Just stick the ret); part onto the same line and be done with it.

:resend that I mistakenly sent only to Mark, with a couple more comments

I disagree.

Look at the long line wrapping style of the rest of the file
and most of the kernel source. No additional arguments to
functions are generally used after column 80.

If it's ever agreed that all lines > 80 cols are OK or
new args after column 80 are OK, then sure. Until then,
if you want it, you should do it.

cheers, Joe

$ grep -A 1 "\bdev_" drivers/ata/pata_at91.c
dev_dbg(dev, "ATA timings: nrd_setup = %lu nrd_pulse = %lu nrd_cycle = %lu\n",
nrd_setup, nrd_pulse, read_cycle);
dev_dbg(dev, "ATA timings: nwe_setup = %lu nwe_pulse = %lu nwe_cycle = %lu\n",
nwe_setup, nwe_pulse, write_cycle);
dev_dbg(dev, "ATA timings: ncs_read_setup = %lu ncs_read_pulse = %lu\n",
ncs_read_setup, ncs_read_pulse);
dev_dbg(dev, "ATA timings: ncs_write_setup = %lu ncs_write_pulse = %lu\n",
ncs_write_setup, ncs_write_pulse);
--


dev_warn(ap->dev, "Failed to compute ATA timing %d, set PIO_0 timing\n",

ret);
--
dev_err(&pdev->dev, "invalid number of resources\n");
return -EINVAL;
--
dev_err(dev, "failed to get mem resource\n");
return -EINVAL;
--
dev_err(dev, "failed to allocate memory for private data\n");
return -ENOMEM;
--
dev_err(dev, "failed to get access to mck clock\n");
return -ENODEV;
--
dev_err(dev, "failed to map IO base\n");
ret = -ENOMEM;
--
dev_err(dev, "failed to map CTL base\n");
ret = -ENOMEM;
--
struct ata_host *host = dev_get_drvdata(&pdev->dev);
struct at91_ide_info *info;

Florian Tobias Schandinat

unread,
Feb 3, 2010, 5:40:03 PM2/3/10
to
Hi,

Joe Perches schrieb:


> String constants that are continued on subsequent lines with \
> are not good.

I agree.

> Signed-off-by: Joe Perches <j...@perches.com>

According to how I understand CodingStyle this is the way those things
should be done.

Acked-by: Florian Tobias Schandinat <FlorianS...@gmx.de>

Andrew, can you please take care of this patch?


Thanks,

Florian Tobias Schandinat

--

Krzysztof Halasa

unread,
Feb 3, 2010, 8:50:02 PM2/3/10
to
Joe Perches <j...@perches.com> writes:

> If it's ever agreed that all lines > 80 cols are OK or
> new args after column 80 are OK, then sure.

It's been already agreed they are OK for any code.
There is no problem with complexity there, is there?
--
Krzysztof Halasa

Joe Perches

unread,
Feb 3, 2010, 10:40:01 PM2/3/10
to
On Thu, 2010-02-04 at 02:39 +0100, Krzysztof Halasa wrote:
> Joe Perches <j...@perches.com> writes:
> It's been already agreed they are OK for any code.
> There is no problem with complexity there, is there?

There's no complexity problem.
You're welcome to do it however you chose.
Just as I did.

cheers, Joe

Mark Lord

unread,
Feb 4, 2010, 12:10:02 AM2/4/10
to
Joe Perches wrote:
> On Wed, 2010-02-03 at 12:21 -0500, Mark Lord wrote:
>> Joe Perches wrote:
>>> String constants that are continued on subsequent lines with \
>>> are not good.
> []
>>> - dev_warn(ap->dev, "Failed to compute ATA timing %d, \
>>> - set PIO_0 timing\n", ret);
>>> + dev_warn(ap->dev, "Failed to compute ATA timing %d, set PIO_0 timing\n",
>>> + ret);
>> NAK. Just stick the ret); part onto the same line and be done with it.
>
> :resend that I mistakenly sent only to Mark, with a couple more comments
>
> I disagree.
>
> Look at the long line wrapping style of the rest of the file
> and most of the kernel source. No additional arguments to
> functions are generally used after column 80.
>
> If it's ever agreed that all lines > 80 cols are OK or
> new args after column 80 are OK, then sure. Until then,
> if you want it, you should do it.
..

You are the code nanny trying to get this patch in. Not me.

-ml

Joe Perches

unread,
Feb 4, 2010, 12:20:01 AM2/4/10
to
On Wed, 2010-02-03 at 23:59 -0500, Mark Lord wrote:
> You are the code nanny trying to get this patch in. Not me.

Nope, I'm just the guy that identified the defect
and proposed a patch. You can fix it differently
if you want. Jeff could fix it differently as well.
I don't care that much. I don't own a device that
uses this controller. I'll never see the message.
Perhaps no one ever will.

Maybe Sergey Matyukevich, the original author, wants
to fix it up in another way.
http://patchwork.kernel.org/patch/76246/

cheers, Joe

Andrew Morton

unread,
Feb 4, 2010, 12:20:01 AM2/4/10
to
On Wed, 03 Feb 2010 23:59:47 -0500 Mark Lord <ker...@teksavvy.com> wrote:

> Joe Perches wrote:
> > On Wed, 2010-02-03 at 12:21 -0500, Mark Lord wrote:
> >> Joe Perches wrote:
> >>> String constants that are continued on subsequent lines with \
> >>> are not good.
> > []
> >>> - dev_warn(ap->dev, "Failed to compute ATA timing %d, \
> >>> - set PIO_0 timing\n", ret);
> >>> + dev_warn(ap->dev, "Failed to compute ATA timing %d, set PIO_0 timing\n",
> >>> + ret);
> >> NAK. Just stick the ret); part onto the same line and be done with it.
> >
> > :resend that I mistakenly sent only to Mark, with a couple more comments
> >
> > I disagree.
> >
> > Look at the long line wrapping style of the rest of the file
> > and most of the kernel source. No additional arguments to
> > functions are generally used after column 80.
> >
> > If it's ever agreed that all lines > 80 cols are OK or
> > new args after column 80 are OK, then sure. Until then,
> > if you want it, you should do it.
> ..
>
> You are the code nanny trying to get this patch in. Not me.
>

It's a runtime bug. Are "code nannies" supposed to "try" to fix
runtime bugs in libata while others just sit there and insult them?

Mark Lord

unread,
Feb 4, 2010, 9:10:02 AM2/4/10
to
Andrew Morton wrote:
> On Wed, 03 Feb 2010 23:59:47 -0500 Mark Lord <ker...@teksavvy.com> wrote:
>> Joe Perches wrote:
>>> If it's ever agreed that all lines > 80 cols are OK or
>>> new args after column 80 are OK, then sure. Until then,
>>> if you want it, you should do it.
>> ..
>>
>> You are the code nanny trying to get this patch in. Not me.
>>
>
> It's a runtime bug. Are "code nannies" supposed to "try" to fix
> runtime bugs in libata while others just sit there and insult them?
..

Oh, there's a service affecting bug there?
Well, I guess that means we should accept a shoddy patch, then. :)

But seriously, I didn't mean anything bad by it, just that
it ought to be simple for the original sender to just reformat it
and resubmit. Just like everyone else here does on a daily basis.

No point in making the code harder to read/understand for no good reason.

But if these patches are robot generated (by a code scan tool or something),
then the original sender (Joe) might have many others in the queue
throughout the kernel, which could explain the reluctance here.

Really, though the best solution would be for Jeff to just edit
that one line as he integrates the patch, like Linus would do.
I imagine Jeff's already done that by now. ;)

Cheers

Mark Lord

unread,
Feb 4, 2010, 9:10:01 AM2/4/10
to
Joe Perches wrote:
> If it's ever agreed that all lines > 80 cols are OK or
> new args after column 80 are OK, then sure. Until then,
> if you want it, you should do it.
..

In case you missed it, it *has* already been "agreed" that
there's nothing magical about 80 anymore.

-------- Original Message --------
Subject: Re: [PATCH] ptrace: checkpatch fixes
Date: Wed, 8 Apr 2009 10:19:36 -0700 (PDT)
From: Linus Torvalds <torv...@linux-foundation.org>
To: Roland McGrath <rol...@redhat.com>
CC: Andrew Morton <ak...@linux-foundation.org>, linux-...@vger.kernel.org
References: <200904080621...@magilla.sf.frob.com>


On Tue, 7 Apr 2009, Roland McGrath wrote:
>
> This fixes all the checkpatch --file complaints about kernel/ptrace.c
> and also removes an unused #include. I've verified that there are no
> changes to the compiled code on x86_64.

Please don't bother with that insane "line length" option when using
"--file". At least not if the "fix" is to just mindlessly split the line.
That is _never_ a fix.

Changes like these:

> -int ptrace_readdata(struct task_struct *tsk, unsigned long src, char __user *dst, int len)
> +int ptrace_readdata(struct task_struct *tsk, unsigned long src,
> + char __user *dst, int len)

> -int ptrace_writedata(struct task_struct *tsk, char __user *src, unsigned long dst, int len)
> +int ptrace_writedata(struct task_struct *tsk, char __user *src,
> + unsigned long dst, int len)

> case PTRACE_GETEVENTMSG:
> - ret = put_user(child->ptrace_message, (unsigned long __user *) data);
> + ret = put_user(child->ptrace_message,
> + (unsigned long __user *) data);

just make the code harder to 'grep'.

Yes, at some point you have to split lines, but that point is not 80
columns any more. The advantage of getting the whole line when grepping
for function names much outweighs the downside of somebody using those
old 80x24 green phosphorous vt52's.

[ The same thing very much goes for complex if-statements etc. If
people can't stand the long lines, the primary solution would be to
turn a complex conditional into a helper inline functions, or to fix
excessive indentation by splitting up functions.

In the above case, the last one could perhaps have been handled
creating a new variable for and moving the cast to the initialiser,
for example. Is it worth it to avoid a 85-column line? Probably not.

And some lines just end up long. I think 100 characters may be a
more reasonable limit for "too long", but quite frankly, it depends on
the line.

So I think 'checkpatch' is pure crap in this area, and I've told
people so before, and they keep telling me that it has relaxed it's
idiotic warnings, but that is apparently just a lie. ]

Oh well. If I actually read perl, I could parse what the hell those
80-character rules are in checkpath. It already has random "it's ok if
X" stuff. But it never seems to really have any "oh, but splitting is
worse" logic.

Linus

Avi Kivity

unread,
Feb 7, 2010, 6:50:01 AM2/7/10
to
On 02/02/2010 09:22 AM, Joe Perches wrote:
> String constants that are continued on subsequent lines with \
> are not good.
>

Applied, thanks.

--
error compiling committee.c: too many arguments to function

0 new messages