* Re: [PATCH v2] pata_imx: print error message on platform_get_irq failure
From: Vladimir Zapolskiy @ 2017-07-01 9:00 UTC (permalink / raw)
To: Gustavo A. R. Silva, Bartlomiej Zolnierkiewicz, Tejun Heo
Cc: linux-ide, linux-kernel
In-Reply-To: <20170630213056.GA12695@embeddedgus>
Hello Gustavo,
On 07/01/2017 12:30 AM, Gustavo A. R. Silva wrote:
> Print error message on platform_get_irq failure before return.
>
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> ---
> Changes in v2:
> Print the return value of platform_get_irq on failure.
>
> drivers/ata/pata_imx.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/ata/pata_imx.c b/drivers/ata/pata_imx.c
> index d4caa23..66fb1ab 100644
> --- a/drivers/ata/pata_imx.c
> +++ b/drivers/ata/pata_imx.c
> @@ -132,9 +132,10 @@ static int pata_imx_probe(struct platform_device *pdev)
> int ret;
>
> irq = platform_get_irq(pdev, 0);
> - if (irq < 0)
> + if (irq < 0) {
> + dev_err(&pdev->dev, "failed to get IRQ: %d\n", irq);
> return irq;
> -
> + }
> priv = devm_kzalloc(&pdev->dev,
> sizeof(struct pata_imx_priv), GFP_KERNEL);
> if (!priv)
>
this patch is wrong, I've explained why at https://lkml.org/lkml/2017/6/30/144
Please handle -EPROBE_DEFER case, when your change adds the second (redundant)
error level message printed to the kernel log.
--
With best wishes,
Vladimir
^ permalink raw reply
* (unknown),
From: beautyink @ 2017-07-03 4:44 UTC (permalink / raw)
To: linux-ide
[-- Attachment #1: EMAIL_27450398_linux-ide.zip --]
[-- Type: application/zip, Size: 3158 bytes --]
^ permalink raw reply
* (unknown),
From: mitch_128 @ 2017-07-03 12:43 UTC (permalink / raw)
To: linux-ide
[-- Attachment #1: EMAIL_94858129_linux-ide.zip --]
[-- Type: application/zip, Size: 3177 bytes --]
^ permalink raw reply
* [PATCH 1/2] libata: Introduce ata_platform_shutdown_one()
From: Nate Watterson @ 2017-07-03 18:02 UTC (permalink / raw)
To: Tejun Heo, linux-ide, linux-kernel; +Cc: Nate Watterson
In similar fashion to how ata_platform_remove_one() is used, this
newly introduced method can be used by platform ata drivers to get
basic shutdown functionality (stopping host DMA and interrupts).
Signed-off-by: Nate Watterson <nwatters@codeaurora.org>
---
drivers/ata/libata-core.c | 20 ++++++++++++++++++++
include/linux/libata.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index e157a0e..537932e 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6702,6 +6702,25 @@ int ata_platform_remove_one(struct platform_device *pdev)
return 0;
}
+/**
+ * ata_platform_shutdown_one - Platform layer callback for device shutdown
+ * @pdev: Platform device being shutdown
+ *
+ * Platform layer indicates to libata via this hook that shutdown is
+ * in progress and the input device should be quiesced. Functionally this
+ * is equivalent to ata_platform_remove_one(), however devres_release_all()
+ * is not called on the shutdown path as it is for remove so releasing the
+ * resources associated with the device must instead be initiated directly.
+ */
+void ata_platform_shutdown_one(struct platform_device *pdev)
+{
+ struct ata_host *host = platform_get_drvdata(pdev);
+
+ ata_host_detach(host);
+ devres_release(&pdev->dev, ata_host_stop, NULL, NULL);
+ devres_release(&pdev->dev, ata_host_release, NULL, NULL);
+}
+
static int __init ata_parse_force_one(char **cur,
struct ata_force_ent *force_ent,
const char **reason)
@@ -7222,6 +7241,7 @@ void ata_print_version(const struct device *dev, const char *version)
#endif /* CONFIG_PCI */
EXPORT_SYMBOL_GPL(ata_platform_remove_one);
+EXPORT_SYMBOL_GPL(ata_platform_shutdown_one);
EXPORT_SYMBOL_GPL(__ata_ehi_push_desc);
EXPORT_SYMBOL_GPL(ata_ehi_push_desc);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index c9a69fc..6c01f23f 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1240,6 +1240,7 @@ struct pci_bits {
struct platform_device;
extern int ata_platform_remove_one(struct platform_device *pdev);
+extern void ata_platform_shutdown_one(struct platform_device *pdev);
/*
* ACPI - drivers/ata/libata-acpi.c
--
Qualcomm Datacenter Technologies, Inc. on behalf of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux
Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH 2/2] ata: ahci_platform: Handle shutdown with ata_platform_shutdown_one()
From: Nate Watterson @ 2017-07-03 18:02 UTC (permalink / raw)
To: Hans de Goede, Tejun Heo, linux-ide, linux-kernel; +Cc: Nate Watterson
In-Reply-To: <1499104962-17498-1-git-send-email-nwatters@codeaurora.org>
To avoid interfering with a new kernel started using kexec, quiesce
host controller DMA interrupts during driver shutdown with
ata_platform_remove_one().
Change-Id: Iedee7f7ba12172b3d34796a3e3b92dbb72d4ed9c
Signed-off-by: Nate Watterson <nwatters@codeaurora.org>
---
drivers/ata/ahci_platform.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 62a04c8..ca25b0d 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -93,6 +93,7 @@ static SIMPLE_DEV_PM_OPS(ahci_pm_ops, ahci_platform_suspend,
static struct platform_driver ahci_driver = {
.probe = ahci_probe,
.remove = ata_platform_remove_one,
+ .shutdown = ata_platform_shutdown_one,
.driver = {
.name = DRV_NAME,
.of_match_table = ahci_of_match,
--
Qualcomm Datacenter Technologies, Inc. on behalf of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux
Foundation Collaborative Project.
^ permalink raw reply related
* Re: [PATCH 1/2] libata: Introduce ata_platform_shutdown_one()
From: Tejun Heo @ 2017-07-05 14:50 UTC (permalink / raw)
To: Nate Watterson; +Cc: linux-ide, linux-kernel
In-Reply-To: <1499104962-17498-1-git-send-email-nwatters@codeaurora.org>
On Mon, Jul 03, 2017 at 02:02:41PM -0400, Nate Watterson wrote:
> In similar fashion to how ata_platform_remove_one() is used, this
> newly introduced method can be used by platform ata drivers to get
> basic shutdown functionality (stopping host DMA and interrupts).
>
> Signed-off-by: Nate Watterson <nwatters@codeaurora.org>
> ---
> drivers/ata/libata-core.c | 20 ++++++++++++++++++++
> include/linux/libata.h | 1 +
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index e157a0e..537932e 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -6702,6 +6702,25 @@ int ata_platform_remove_one(struct platform_device *pdev)
> return 0;
> }
>
> +/**
> + * ata_platform_shutdown_one - Platform layer callback for device shutdown
> + * @pdev: Platform device being shutdown
> + *
> + * Platform layer indicates to libata via this hook that shutdown is
> + * in progress and the input device should be quiesced. Functionally this
^^^^^
> + * is equivalent to ata_platform_remove_one(), however devres_release_all()
> + * is not called on the shutdown path as it is for remove so releasing the
> + * resources associated with the device must instead be initiated directly.
I can't easily understand the above comment. Can you please try to
rephrase it so that it's clear why and when it's used?
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH 2/2] ata: ahci_platform: Handle shutdown with ata_platform_shutdown_one()
From: Tejun Heo @ 2017-07-05 14:51 UTC (permalink / raw)
To: Nate Watterson; +Cc: Hans de Goede, linux-ide, linux-kernel
In-Reply-To: <1499104962-17498-2-git-send-email-nwatters@codeaurora.org>
On Mon, Jul 03, 2017 at 02:02:42PM -0400, Nate Watterson wrote:
> To avoid interfering with a new kernel started using kexec, quiesce
> host controller DMA interrupts during driver shutdown with
> ata_platform_remove_one().
>
> Change-Id: Iedee7f7ba12172b3d34796a3e3b92dbb72d4ed9c
> Signed-off-by: Nate Watterson <nwatters@codeaurora.org>
Please drop the Change-Id when posting upstream. Also, it probably
would be better to roll this into the previous patch.
Thanks.
--
tejun
^ permalink raw reply
* [GIT PULL] libata changes for v4.13-rc1
From: Tejun Heo @ 2017-07-05 19:22 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-kernel, linux-ide, Bartlomiej Zolnierkiewicz, Hans de Goede,
Christoph Hellwig, Minwoo Im, Linus Walleij
Hello, Linus.
* Christoph added support for TCG OPAL self encrypting disks.
* Minwoo added support for ATA PASS-THROUGH(32).
* Linus Walleij removed spurious drvdata assignments in some drivers.
* Support for a few new device and other fixes.
Thanks.
The following changes since commit a95cfad947d5f40cfbf9ad3019575aac1d8ac7a6:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2017-05-15 15:50:49 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata.git for-4.13
for you to fetch changes up to d80210f25ff0050245556bb8ce84d280d8fa4ca7:
sd: add support for TCG OPAL self encrypting disks (2017-06-29 10:21:15 -0400)
----------------------------------------------------------------
Arnd Bergmann (1):
ata: ftide010: fix resource printing
Arvind Yadav (2):
ata: pata_octeon_cf: make of_device_ids const.
ata: sata_rcar: make of_device_ids const.
Bhumika Goyal (1):
ata: declare ata_port_info structures as const
Christoph Hellwig (6):
libata: move ata_read_log_page to libata-core.c
libata: factor out a ata_log_supported helper
libata: clarify log page naming / grouping
libata: factor out a ata_identify_page_supported helper
libata: implement SECURITY PROTOCOL IN/OUT
sd: add support for TCG OPAL self encrypting disks
Doug Berger (3):
libata: Add the AHCI_HFLAG_YES_ALPM flag
libata: Add the AHCI_HFLAG_NO_WRITE_TO_RO flag
ata: ahci_brcm: Avoid writing to read-only registers
Joe Perches (1):
libata: Convert bare printks to pr_cont
Linus Walleij (9):
ata: bf54x: cut drvdata assignment
ata: ep93xx: cut drvdata assignment
ata: dwc_460ex: cut drvdata assignment
ata: rb532_cf: cut drvdata assignment
ata: samsung_cf: cut drvdata assignment
ata: sata_fsl: cut drvdata assignment
ata: Add DT bindings for Faraday Technology FTIDE010
ata: Add DT bindings for the Gemini SATA bridge
ata: Add driver for Faraday Technology FTIDE010
Mauro Carvalho Chehab (2):
libata: fix identation on a kernel-doc markup
ata: update references for libata documentation
Minwoo Im (3):
libahci: wrong comments in ahci_do_softreset()
libata: make the function name in comment match the actual function
libata: Support for an ATA PASS-THROUGH(32) command.
Ondrej Zary (1):
sata_via: Enable optional hotplug on VT6420
Shawn Lin (1):
ahci: Add Device ID for ASMedia 1061R and 1062R
Tejun Heo (1):
libata: fix build warning from unused goto label
Tycho Andersen (1):
ata-sff: always map page before data transfer
Yuantian Tang (1):
ahci: qoriq: add ls1088a platforms support
.../bindings/ata/cortina,gemini-sata-bridge.txt | 55 ++
.../devicetree/bindings/ata/faraday,ftide010.txt | 38 ++
MAINTAINERS | 9 +
drivers/ata/Kconfig | 21 +
drivers/ata/Makefile | 2 +
drivers/ata/acard-ahci.c | 2 +-
drivers/ata/ahci.c | 4 +-
drivers/ata/ahci.h | 5 +-
drivers/ata/ahci_brcm.c | 12 +-
drivers/ata/ahci_qoriq.c | 14 +
drivers/ata/ata_piix.c | 2 +-
drivers/ata/libahci.c | 14 +-
drivers/ata/libata-core.c | 216 +++++---
drivers/ata/libata-eh.c | 66 +--
drivers/ata/libata-scsi.c | 202 ++++++--
drivers/ata/libata-sff.c | 46 +-
drivers/ata/libata.h | 6 +-
drivers/ata/pata_bf54x.c | 2 -
drivers/ata/pata_ep93xx.c | 1 -
drivers/ata/pata_ftide010.c | 567 +++++++++++++++++++++
drivers/ata/pata_octeon_cf.c | 2 +-
drivers/ata/pata_pdc2027x.c | 2 +-
drivers/ata/pata_rb532_cf.c | 2 -
drivers/ata/pata_rdc.c | 2 +-
drivers/ata/pata_samsung_cf.c | 2 -
drivers/ata/pata_sch.c | 2 +-
drivers/ata/pdc_adma.c | 2 +-
drivers/ata/sata_dwc_460ex.c | 1 -
drivers/ata/sata_fsl.c | 2 -
drivers/ata/sata_gemini.c | 438 ++++++++++++++++
drivers/ata/sata_gemini.h | 21 +
drivers/ata/sata_inic162x.c | 2 +-
drivers/ata/sata_nv.c | 2 +-
drivers/ata/sata_promise.c | 2 +-
drivers/ata/sata_promise.h | 2 +-
drivers/ata/sata_qstor.c | 2 +-
drivers/ata/sata_rcar.c | 2 +-
drivers/ata/sata_sil.c | 2 +-
drivers/ata/sata_sis.c | 2 +-
drivers/ata/sata_svw.c | 2 +-
drivers/ata/sata_sx4.c | 2 +-
drivers/ata/sata_uli.c | 2 +-
drivers/ata/sata_via.c | 25 +-
drivers/ata/sata_vsc.c | 2 +-
drivers/scsi/sd.c | 53 +-
drivers/scsi/sd.h | 2 +
include/linux/ata.h | 13 +-
include/linux/libata.h | 3 +-
include/scsi/scsi_device.h | 1 +
include/scsi/scsi_proto.h | 1 +
50 files changed, 1618 insertions(+), 264 deletions(-)
create mode 100644 Documentation/devicetree/bindings/ata/cortina,gemini-sata-bridge.txt
create mode 100644 Documentation/devicetree/bindings/ata/faraday,ftide010.txt
create mode 100644 drivers/ata/pata_ftide010.c
create mode 100644 drivers/ata/sata_gemini.c
create mode 100644 drivers/ata/sata_gemini.h
--
tejun
^ permalink raw reply
* [PATCH v2 7/9] drivers/ata: Replace spin_unlock_wait() with lock/unlock pair
From: Paul E. McKenney @ 2017-07-05 23:31 UTC (permalink / raw)
To: linux-kernel
Cc: netfilter-devel, netdev, oleg, akpm, mingo, dave, manfred, tj,
arnd, linux-arch, will.deacon, peterz, stern, parri.andrea,
torvalds, Paul E. McKenney, linux-ide
In-Reply-To: <20170705232955.GA15992@linux.vnet.ibm.com>
There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair. This commit therefore eliminates the spin_unlock_wait() call and
associated else-clause and hoists the then-clause's lock and unlock out of
the "if" statement. This should be safe from a performance perspective
because according to Tejun there should be few if any drivers that don't
set their own error handler.
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Acked-by: Tejun Heo <tj@kernel.org>
Cc: <linux-ide@vger.kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
drivers/ata/libata-eh.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index ef68232b5222..779f6f18c1f4 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -645,12 +645,11 @@ void ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap,
* completions are honored. A scmd is determined to have
* timed out iff its associated qc is active and not failed.
*/
+ spin_lock_irqsave(ap->lock, flags);
if (ap->ops->error_handler) {
struct scsi_cmnd *scmd, *tmp;
int nr_timedout = 0;
- spin_lock_irqsave(ap->lock, flags);
-
/* This must occur under the ap->lock as we don't want
a polled recovery to race the real interrupt handler
@@ -700,12 +699,11 @@ void ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap,
if (nr_timedout)
__ata_port_freeze(ap);
- spin_unlock_irqrestore(ap->lock, flags);
/* initialize eh_tries */
ap->eh_tries = ATA_EH_MAX_TRIES;
- } else
- spin_unlock_wait(ap->lock);
+ }
+ spin_unlock_irqrestore(ap->lock, flags);
}
EXPORT_SYMBOL(ata_scsi_cmd_error_handler);
--
2.5.2
^ permalink raw reply related
* [PATCH v3 7/9] drivers/ata: Replace spin_unlock_wait() with lock/unlock pair
From: Paul E. McKenney @ 2017-07-07 19:28 UTC (permalink / raw)
To: linux-kernel
Cc: netfilter-devel, netdev, oleg, akpm, mingo, dave, manfred, tj,
arnd, linux-arch, will.deacon, peterz, stern, parri.andrea,
torvalds, Paul E. McKenney, linux-ide
In-Reply-To: <20170707192704.GA6144@linux.vnet.ibm.com>
There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair. This commit therefore eliminates the spin_unlock_wait() call and
associated else-clause and hoists the then-clause's lock and unlock out of
the "if" statement. This should be safe from a performance perspective
because according to Tejun there should be few if any drivers that don't
set their own error handler.
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Acked-by: Tejun Heo <tj@kernel.org>
Cc: <linux-ide@vger.kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
drivers/ata/libata-eh.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index ef68232b5222..779f6f18c1f4 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -645,12 +645,11 @@ void ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap,
* completions are honored. A scmd is determined to have
* timed out iff its associated qc is active and not failed.
*/
+ spin_lock_irqsave(ap->lock, flags);
if (ap->ops->error_handler) {
struct scsi_cmnd *scmd, *tmp;
int nr_timedout = 0;
- spin_lock_irqsave(ap->lock, flags);
-
/* This must occur under the ap->lock as we don't want
a polled recovery to race the real interrupt handler
@@ -700,12 +699,11 @@ void ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap,
if (nr_timedout)
__ata_port_freeze(ap);
- spin_unlock_irqrestore(ap->lock, flags);
/* initialize eh_tries */
ap->eh_tries = ATA_EH_MAX_TRIES;
- } else
- spin_unlock_wait(ap->lock);
+ }
+ spin_unlock_irqrestore(ap->lock, flags);
}
EXPORT_SYMBOL(ata_scsi_cmd_error_handler);
--
2.5.2
^ permalink raw reply related
* [PATCH] libata: Cleanup ata_read_log_page()
From: Damien Le Moal @ 2017-07-10 5:45 UTC (permalink / raw)
To: linux-ide, Tejun Heo; +Cc: Hannes Reinecke
The warning message "READ LOG DMA EXT failed, trying unqueued" in
ata_read_log_page() as well as the macro name ATA_HORKAGE_NO_NCQ_LOG
are confusing: the command READ LOG DMA EXT is not an queued NCQ command
unless it is encapsulated in a RECEIVE FPDMA QUEUED command.
>From ACS-4 READ LOG DMA EXT description:
"The device processes the READ LOG DMA EXT command in the NCQ feature
set environment (see 4.13.6) if the READ LOG DMA EXT command is
encapsulated in a RECEIVE FPDMA QUEUED command (see 7.30) with the
inputs encapsulated as shown in 7.23.6."
To avoid confusion, fix the warning messsage to mention switching to PIO and
not "unqueued" and rename the macro ATA_HORKAGE_NO_NCQ_LOG to
ATA_HORKAGE_NO_DMA_LOG.
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
drivers/ata/libata-eh.c | 6 +++---
include/linux/libata.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index ef68232..f3f5125 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1523,7 +1523,7 @@ unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
retry:
ata_tf_init(dev, &tf);
if (dev->dma_mode && ata_id_has_read_log_dma_ext(dev->id) &&
- !(dev->horkage & ATA_HORKAGE_NO_NCQ_LOG)) {
+ !(dev->horkage & ATA_HORKAGE_NO_DMA_LOG)) {
tf.command = ATA_CMD_READ_LOG_DMA_EXT;
tf.protocol = ATA_PROT_DMA;
dma = true;
@@ -1542,8 +1542,8 @@ unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
buf, sectors * ATA_SECT_SIZE, 0);
if (err_mask && dma) {
- dev->horkage |= ATA_HORKAGE_NO_NCQ_LOG;
- ata_dev_warn(dev, "READ LOG DMA EXT failed, trying unqueued\n");
+ dev->horkage |= ATA_HORKAGE_NO_DMA_LOG;
+ ata_dev_warn(dev, "READ LOG DMA EXT failed, trying PIO\n");
goto retry;
}
diff --git a/include/linux/libata.h b/include/linux/libata.h
index c9a69fc..4718e85 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -434,7 +434,7 @@ enum {
ATA_HORKAGE_NOLPM = (1 << 20), /* don't use LPM */
ATA_HORKAGE_WD_BROKEN_LPM = (1 << 21), /* some WDs have broken LPM */
ATA_HORKAGE_ZERO_AFTER_TRIM = (1 << 22),/* guarantees zero after trim */
- ATA_HORKAGE_NO_NCQ_LOG = (1 << 23), /* don't use NCQ for log read */
+ ATA_HORKAGE_NO_DMA_LOG = (1 << 23), /* don't use DMA for log read */
ATA_HORKAGE_NOTRIM = (1 << 24), /* don't use TRIM */
ATA_HORKAGE_MAX_SEC_1024 = (1 << 25), /* Limit max sects to 1024 */
--
2.9.4
^ permalink raw reply related
* Re: [PATCH] libata: Cleanup ata_read_log_page()
From: Hannes Reinecke @ 2017-07-10 7:10 UTC (permalink / raw)
To: Damien Le Moal, linux-ide, Tejun Heo
In-Reply-To: <20170710054520.24213-1-damien.lemoal@wdc.com>
On 07/10/2017 07:45 AM, Damien Le Moal wrote:
> The warning message "READ LOG DMA EXT failed, trying unqueued" in
> ata_read_log_page() as well as the macro name ATA_HORKAGE_NO_NCQ_LOG
> are confusing: the command READ LOG DMA EXT is not an queued NCQ command
> unless it is encapsulated in a RECEIVE FPDMA QUEUED command.
> From ACS-4 READ LOG DMA EXT description:
>
> "The device processes the READ LOG DMA EXT command in the NCQ feature
> set environment (see 4.13.6) if the READ LOG DMA EXT command is
> encapsulated in a RECEIVE FPDMA QUEUED command (see 7.30) with the
> inputs encapsulated as shown in 7.23.6."
>
> To avoid confusion, fix the warning messsage to mention switching to PIO and
> not "unqueued" and rename the macro ATA_HORKAGE_NO_NCQ_LOG to
> ATA_HORKAGE_NO_DMA_LOG.
>
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
> drivers/ata/libata-eh.c | 6 +++---
> include/linux/libata.h | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
You are correct.
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* [PATCH] ata: fix gemini Kconfig dependencies
From: Arnd Bergmann @ 2017-07-10 8:51 UTC (permalink / raw)
To: Tejun Heo
Cc: Arnd Bergmann, Bartlomiej Zolnierkiewicz, Geert Uytterhoeven,
Linus Walleij, linux-ide, linux-kernel
We cannot build the new ftide010 code without also building the faraday
sata bridge driver:
drivers/ata/pata_ftide010.o: In function `pata_ftide010_probe':
pata_ftide010.c:(.text+0x2b8): undefined reference to `gemini_sata_bridge_get'
pata_ftide010.c:(.text+0x32c): undefined reference to `gemini_sata_get_muxmode'
pata_ftide010.c:(.text+0x358): undefined reference to `gemini_sata_bridge_enabled'
drivers/ata/pata_ftide010.o: In function `pata_ftide010_gemini_port_stop':
pata_ftide010.c:(.text+0x520): undefined reference to `gemini_sata_stop_bridge'
drivers/ata/pata_ftide010.o: In function `pata_ftide010_gemini_port_start':
pata_ftide010.c:(.text+0x5bc): undefined reference to `gemini_sata_start_bridge'
This adjusts the Kconfig dependencies accordingly.
Fixes: be4e456ed3a5 ("ata: Add driver for Faraday Technology FTIDE010")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/ata/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 948fc86980a1..363fc5330c21 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -215,7 +215,7 @@ config SATA_FSL
config SATA_GEMINI
tristate "Gemini SATA bridge support"
- depends on PATA_FTIDE010
+ depends on ARCH_GEMINI || COMPILE_TEST
default ARCH_GEMINI
help
This enabled support for the FTIDE010 to SATA bridge
@@ -613,7 +613,7 @@ config PATA_FTIDE010
tristate "Faraday Technology FTIDE010 PATA support"
depends on OF
depends on ARM
- default ARCH_GEMINI
+ depends on SATA_GEMINI
help
This option enables support for the Faraday FTIDE010
PATA controller found in the Cortina Gemini SoCs.
--
2.9.0
^ permalink raw reply related
* (unknown),
From: lucia.germino @ 2017-07-10 12:51 UTC (permalink / raw)
To: linux-ide
[-- Attachment #1: 192119470.zip --]
[-- Type: application/zip, Size: 3546 bytes --]
^ permalink raw reply
* Re: [PATCH] libata: Cleanup ata_read_log_page()
From: Tejun Heo @ 2017-07-10 17:38 UTC (permalink / raw)
To: Damien Le Moal; +Cc: linux-ide, Hannes Reinecke
In-Reply-To: <20170710054520.24213-1-damien.lemoal@wdc.com>
Hello,
On Mon, Jul 10, 2017 at 02:45:20PM +0900, Damien Le Moal wrote:
> The warning message "READ LOG DMA EXT failed, trying unqueued" in
> ata_read_log_page() as well as the macro name ATA_HORKAGE_NO_NCQ_LOG
> are confusing: the command READ LOG DMA EXT is not an queued NCQ command
> unless it is encapsulated in a RECEIVE FPDMA QUEUED command.
> From ACS-4 READ LOG DMA EXT description:
>
> "The device processes the READ LOG DMA EXT command in the NCQ feature
> set environment (see 4.13.6) if the READ LOG DMA EXT command is
> encapsulated in a RECEIVE FPDMA QUEUED command (see 7.30) with the
> inputs encapsulated as shown in 7.23.6."
>
> To avoid confusion, fix the warning messsage to mention switching to PIO and
> not "unqueued" and rename the macro ATA_HORKAGE_NO_NCQ_LOG to
> ATA_HORKAGE_NO_DMA_LOG.
>
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Applied to libata/for-4.13-fixes.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH] ata: fix gemini Kconfig dependencies
From: Tejun Heo @ 2017-07-10 17:43 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Bartlomiej Zolnierkiewicz, Geert Uytterhoeven, Linus Walleij,
linux-ide, linux-kernel
In-Reply-To: <20170710085120.1984293-1-arnd@arndb.de>
On Mon, Jul 10, 2017 at 10:51:02AM +0200, Arnd Bergmann wrote:
> We cannot build the new ftide010 code without also building the faraday
> sata bridge driver:
>
> drivers/ata/pata_ftide010.o: In function `pata_ftide010_probe':
> pata_ftide010.c:(.text+0x2b8): undefined reference to `gemini_sata_bridge_get'
> pata_ftide010.c:(.text+0x32c): undefined reference to `gemini_sata_get_muxmode'
> pata_ftide010.c:(.text+0x358): undefined reference to `gemini_sata_bridge_enabled'
> drivers/ata/pata_ftide010.o: In function `pata_ftide010_gemini_port_stop':
> pata_ftide010.c:(.text+0x520): undefined reference to `gemini_sata_stop_bridge'
> drivers/ata/pata_ftide010.o: In function `pata_ftide010_gemini_port_start':
> pata_ftide010.c:(.text+0x5bc): undefined reference to `gemini_sata_start_bridge'
>
> This adjusts the Kconfig dependencies accordingly.
>
> Fixes: be4e456ed3a5 ("ata: Add driver for Faraday Technology FTIDE010")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied to libata/for-4.13-fixes.
Thanks.
--
tejun
^ permalink raw reply
* [PATCH] ata: sata_rcar: add gen[23] fallback compatibility strings
From: Simon Horman @ 2017-07-11 11:44 UTC (permalink / raw)
To: Tejun Heo
Cc: linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Magnus Damm,
Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
Geert Uytterhoeven, Simon Horman
Add fallback compatibility string for R-Car Gen 2 and 3.
In the case of Renesas R-Car hardware we know that there are generations of
SoCs, e.g. Gen 1 and 2. But beyond that its not clear what the relationship
between IP blocks might be. For example, I believe that r8a7790 is older
than r8a7791 but that doesn't imply that the latter is a descendant of the
former or vice versa.
We can, however, by examining the documentation and behaviour of the
hardware at run-time observe that the current driver implementation appears
to be compatible with the IP blocks on SoCs within a given generation.
For the above reasons and convenience when enabling new SoCs a
per-generation fallback compatibility string scheme being adopted for
drivers for Renesas SoCs.
Signed-off-by: Simon Horman <horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
---
Based on libata/for-next
---
Documentation/devicetree/bindings/ata/sata_rcar.txt | 14 +++++++++++---
drivers/ata/sata_rcar.c | 8 ++++++++
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/ata/sata_rcar.txt b/Documentation/devicetree/bindings/ata/sata_rcar.txt
index 0764f9ab63dc..e20eac7a3087 100644
--- a/Documentation/devicetree/bindings/ata/sata_rcar.txt
+++ b/Documentation/devicetree/bindings/ata/sata_rcar.txt
@@ -1,14 +1,22 @@
* Renesas R-Car SATA
Required properties:
-- compatible : should contain one of the following:
+- compatible : should contain one or more of the following:
- "renesas,sata-r8a7779" for R-Car H1
- ("renesas,rcar-sata" is deprecated)
- "renesas,sata-r8a7790-es1" for R-Car H2 ES1
- "renesas,sata-r8a7790" for R-Car H2 other than ES1
- "renesas,sata-r8a7791" for R-Car M2-W
- "renesas,sata-r8a7793" for R-Car M2-N
- "renesas,sata-r8a7795" for R-Car H3
+ - "renesas,rcar-gen2-sata" for a generic R-Car Gen2 compatible device
+ - "renesas,rcar-gen3-sata" for a generic R-Car Gen3 compatible device
+ - "renesas,rcar-sata" is deprecated
+
+ When compatible with the generic version nodes
+ must list the SoC-specific version corresponding
+ to the platform first followed by the generic
+ version.
+
- reg : address and length of the SATA registers;
- interrupts : must consist of one interrupt specifier.
- clocks : must contain a reference to the functional clock.
@@ -16,7 +24,7 @@ Required properties:
Example:
sata0: sata@ee300000 {
- compatible = "renesas,sata-r8a7791";
+ compatible = "renesas,sata-r8a7791", "renesas,rcar-gen2-sata";
reg = <0 0xee300000 0 0x2000>;
interrupt-parent = <&gic>;
interrupts = <0 105 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index ee9844758736..537d11869069 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -858,6 +858,14 @@ static const struct of_device_id sata_rcar_match[] = {
.compatible = "renesas,sata-r8a7795",
.data = (void *)RCAR_GEN2_SATA
},
+ {
+ .compatible = "renesas,rcar-gen2-sata",
+ .data = (void *)RCAR_GEN2_SATA
+ },
+ {
+ .compatible = "renesas,rcar-gen3-sata",
+ .data = (void *)RCAR_GEN2_SATA
+ },
{ },
};
MODULE_DEVICE_TABLE(of, sata_rcar_match);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] ata: fix gemini Kconfig dependencies
From: Linus Walleij @ 2017-07-11 14:29 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Tejun Heo, Bartlomiej Zolnierkiewicz, Geert Uytterhoeven,
linux-ide, linux-kernel@vger.kernel.org
In-Reply-To: <20170710085120.1984293-1-arnd@arndb.de>
On Mon, Jul 10, 2017 at 10:51 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> We cannot build the new ftide010 code without also building the faraday
> sata bridge driver:
>
> drivers/ata/pata_ftide010.o: In function `pata_ftide010_probe':
> pata_ftide010.c:(.text+0x2b8): undefined reference to `gemini_sata_bridge_get'
> pata_ftide010.c:(.text+0x32c): undefined reference to `gemini_sata_get_muxmode'
> pata_ftide010.c:(.text+0x358): undefined reference to `gemini_sata_bridge_enabled'
> drivers/ata/pata_ftide010.o: In function `pata_ftide010_gemini_port_stop':
> pata_ftide010.c:(.text+0x520): undefined reference to `gemini_sata_stop_bridge'
> drivers/ata/pata_ftide010.o: In function `pata_ftide010_gemini_port_start':
> pata_ftide010.c:(.text+0x5bc): undefined reference to `gemini_sata_start_bridge'
>
> This adjusts the Kconfig dependencies accordingly.
>
> Fixes: be4e456ed3a5 ("ata: Add driver for Faraday Technology FTIDE010")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
My intention was definately to have the bridge driver as a separate add-on
as I suspect some systems use the PATA driver without the SATA bridge.
But we can definately deal with that when those users appear.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] ata: sata_rcar: add gen[23] fallback compatibility strings
From: Sergei Shtylyov @ 2017-07-11 16:23 UTC (permalink / raw)
To: Simon Horman, Tejun Heo
Cc: linux-ide, linux-renesas-soc, Magnus Damm, Rob Herring,
devicetree, Geert Uytterhoeven
In-Reply-To: <1499773460-8930-1-git-send-email-horms+renesas@verge.net.au>
On 07/11/2017 02:44 PM, Simon Horman wrote:
> Add fallback compatibility string for R-Car Gen 2 and 3.
>
> In the case of Renesas R-Car hardware we know that there are generations of
> SoCs, e.g. Gen 1 and 2. But beyond that its not clear what the relationship
> between IP blocks might be. For example, I believe that r8a7790 is older
> than r8a7791 but that doesn't imply that the latter is a descendant of the
> former or vice versa.
>
> We can, however, by examining the documentation and behaviour of the
> hardware at run-time observe that the current driver implementation appears
> to be compatible with the IP blocks on SoCs within a given generation.
>
> For the above reasons and convenience when enabling new SoCs a
> per-generation fallback compatibility string scheme being adopted for
> drivers for Renesas SoCs.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
[...]
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
MBR, Sergei
^ permalink raw reply
* Re: [PATCH] ata: sata_rcar: add gen[23] fallback compatibility strings
From: Geert Uytterhoeven @ 2017-07-11 17:47 UTC (permalink / raw)
To: Simon Horman
Cc: Tejun Heo, linux-ide@vger.kernel.org, Linux-Renesas, Magnus Damm,
Rob Herring, devicetree@vger.kernel.org, Geert Uytterhoeven
In-Reply-To: <1499773460-8930-1-git-send-email-horms+renesas@verge.net.au>
On Tue, Jul 11, 2017 at 1:44 PM, Simon Horman
<horms+renesas@verge.net.au> wrote:
> Add fallback compatibility string for R-Car Gen 2 and 3.
>
> In the case of Renesas R-Car hardware we know that there are generations of
> SoCs, e.g. Gen 1 and 2. But beyond that its not clear what the relationship
> between IP blocks might be. For example, I believe that r8a7790 is older
> than r8a7791 but that doesn't imply that the latter is a descendant of the
> former or vice versa.
>
> We can, however, by examining the documentation and behaviour of the
> hardware at run-time observe that the current driver implementation appears
> to be compatible with the IP blocks on SoCs within a given generation.
>
> For the above reasons and convenience when enabling new SoCs a
> per-generation fallback compatibility string scheme being adopted for
> drivers for Renesas SoCs.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Lots of new warnings with gcc-7.1.1
From: Linus Torvalds @ 2017-07-11 22:35 UTC (permalink / raw)
To: Tejun Heo, Jean Delvare, Guenter Roeck, Bartlomiej Zolnierkiewicz,
Sathya Prakash, James E.J. Bottomley, Greg Kroah-Hartman
Cc: the arch/x86 maintainers, xen-devel, linux-block,
Linux Media Mailing List, IDE-ML, linux-fbdev@vger.kernel.org,
Network Development
[ Very random list of maintainers and mailing lists, at least
partially by number of warnings generated by gcc-7.1.1 that is then
correlated with the get_maintainers script ]
So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1
Which in turn means that my nice clean allmodconfig compile is not an
unholy mess of annoying new warnings.
Normally I hate the stupid new warnings, but this time around they are
actually exactly the kinds of warnings you'd want to see and that are
hard for humans to pick out errors: lots of format errors wrt limited
buffer sizes.
At the same time, many of them *are* annoying. We have various limited
buffers that are limited for a good reason, and some of the format
truncation warnings are about numbers in the range {0-MAX_INT], where
we definitely know that we don't need to worry about the really big
ones.
After all, we're using "snprintf()" for a reason - we *want* to
truncate if the buffer is too small.
But a lot of the warnings look reasonable, and at least the warnings
are nice in how they actually explain why the warning is happening.
Example:
arch/x86/platform/intel-mid/device_libs/platform_max7315.c: In
function ‘max7315_platform_data’:
arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:35:
warning: ‘%d’ directive writing between 1 and 11 bytes into a region
of size 9 [-Wformat-overflow=]
sprintf(base_pin_name, "max7315_%d_base", nr);
^~
arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:26:
note: directive argument in the range [-2147483647, 2147483647]
Yeah, the compiler is technically correct, but we already made sure we
have at most MAX7315_NUM of those adapters, so no, "nr" is really not
going to be a 10-digit number.
So the warning is kind of bogus.
At the same time, others aren't quite as insane, and in many cases the
warnings might be easy to just fix.
And some actually look valid, although they might still require odd input:
net/bluetooth/smp.c: In function ‘le_max_key_size_read’:
net/bluetooth/smp.c:3372:29: warning: ‘snprintf’ output may be
truncated before the last format character [-Wformat-truncation=]
snprintf(buf, sizeof(buf), "%2u\n", SMP_DEV(hdev)->max_key_size);
^~~~~~~
net/bluetooth/smp.c:3372:2: note: ‘snprintf’ output between 4 and 5
bytes into a destination of size 4
yeah, "max_key_size" is unsigned char, but if it's larger than 99 it
really does need 5 bytes for "%2u\n" with the terminating NUL
character.
Of course, the "%2d" implies that people expect it to be < 100, but at
the same time it doesn't sound like a bad idea to just make the buffer
be one byte bigger. So..
Anyway, it would be lovely if some of the more affected developers
would take a look at gcc-7.1.1 warnings. Right now I get about three
*thousand* lines of warnings from a "make allmodconfig" build, which
makes them a bit overwhelming.
I do suspect I'll make "-Wformat-truncation" (as opposed to
"-Wformat-overflow") be a "V=1" kind of warning. But let's see how
many of these we can fix, ok?
Linus
^ permalink raw reply
* Re: Lots of new warnings with gcc-7.1.1
From: Marcel Holtmann @ 2017-07-11 23:54 UTC (permalink / raw)
To: Linus Torvalds
Cc: Tejun Heo, Jean Delvare, Guenter Roeck, Bartlomiej Zolnierkiewicz,
Sathya Prakash, James E.J. Bottomley, Greg Kroah-Hartman,
the arch/x86 maintainers, xen-devel, linux-block,
Linux Media Mailing List, IDE-ML, linux-fbdev@vger.kernel.org,
Network Development
In-Reply-To: <CA+55aFzXz-PxKSJP=hfHD+mfCX4M6+HMacWMkDz7KB8-3y55qw@mail.gmail.com>
Hi Linus,
> At the same time, others aren't quite as insane, and in many cases the
> warnings might be easy to just fix.
>
> And some actually look valid, although they might still require odd input:
>
> net/bluetooth/smp.c: In function ‘le_max_key_size_read’:
> net/bluetooth/smp.c:3372:29: warning: ‘snprintf’ output may be
> truncated before the last format character [-Wformat-truncation=]
> snprintf(buf, sizeof(buf), "%2u\n", SMP_DEV(hdev)->max_key_size);
> ^~~~~~~
> net/bluetooth/smp.c:3372:2: note: ‘snprintf’ output between 4 and 5
> bytes into a destination of size 4
>
> yeah, "max_key_size" is unsigned char, but if it's larger than 99 it
> really does need 5 bytes for "%2u\n" with the terminating NUL
> character.
>
> Of course, the "%2d" implies that people expect it to be < 100, but at
> the same time it doesn't sound like a bad idea to just make the buffer
> be one byte bigger. So..
the Bluetooth specification defines that the Maximum Encryption Key Size shall be in the range 7 to 16 octets. Which is also reflected in these defines:
#define SMP_MIN_ENC_KEY_SIZE 7
#define SMP_MAX_ENC_KEY_SIZE 16
So it is buf[4] since we know it never gets larger than 16. So even in this case the warning is bogus.
I have no problem in increasing it to buf[5] to shut up the compiler, but that is what I would be doing here. I am not fixing an actual bug.
> Anyway, it would be lovely if some of the more affected developers
> would take a look at gcc-7.1.1 warnings. Right now I get about three
> *thousand* lines of warnings from a "make allmodconfig" build, which
> makes them a bit overwhelming.
>
> I do suspect I'll make "-Wformat-truncation" (as opposed to
> "-Wformat-overflow") be a "V=1" kind of warning. But let's see how
> many of these we can fix, ok?
I had to use the -Wno-format-trunction in a few projects since gcc was completely lost. And since we were using snprintf, I saw no point in trying to please gcc with a larger buffer.
Regards
Marcel
^ permalink raw reply
* Re: Lots of new warnings with gcc-7.1.1
From: Guenter Roeck @ 2017-07-12 3:10 UTC (permalink / raw)
To: Linus Torvalds, Tejun Heo, Jean Delvare,
Bartlomiej Zolnierkiewicz, Sathya Prakash, James E.J. Bottomley,
Greg Kroah-Hartman
Cc: the arch/x86 maintainers, xen-devel, linux-block,
Linux Media Mailing List, IDE-ML, linux-fbdev@vger.kernel.org,
Network Development
In-Reply-To: <CA+55aFzXz-PxKSJP=hfHD+mfCX4M6+HMacWMkDz7KB8-3y55qw@mail.gmail.com>
On 07/11/2017 03:35 PM, Linus Torvalds wrote:
> [ Very random list of maintainers and mailing lists, at least
> partially by number of warnings generated by gcc-7.1.1 that is then
> correlated with the get_maintainers script ]
>
> So I upgraded one of my boxes to F26, which upgraded the compiler to gcc-7.1.1
>
> Which in turn means that my nice clean allmodconfig compile is not an
> unholy mess of annoying new warnings.
>
> Normally I hate the stupid new warnings, but this time around they are
> actually exactly the kinds of warnings you'd want to see and that are
> hard for humans to pick out errors: lots of format errors wrt limited
> buffer sizes.
>
> At the same time, many of them *are* annoying. We have various limited
> buffers that are limited for a good reason, and some of the format
> truncation warnings are about numbers in the range {0-MAX_INT], where
> we definitely know that we don't need to worry about the really big
> ones.
>
> After all, we're using "snprintf()" for a reason - we *want* to
> truncate if the buffer is too small.
>
The hwmon warnings are all about supporting no more than 9,999 sensors
(applesmc) to 999,999,999 sensors (scpi) of a given type. Easy "fix" would
be to replace snprintf() with scnprintf(), presumably because gcc doesn't
know about scnprintf(). We could also increase the name buffer size.
But is that really worth it just to silence gcc ?
Guenter
^ permalink raw reply
* Re: Lots of new warnings with gcc-7.1.1
From: Linus Torvalds @ 2017-07-12 3:17 UTC (permalink / raw)
To: Guenter Roeck
Cc: Tejun Heo, Jean Delvare, Bartlomiej Zolnierkiewicz,
Sathya Prakash, James E.J. Bottomley, Greg Kroah-Hartman,
the arch/x86 maintainers, xen-devel, linux-block,
Linux Media Mailing List, IDE-ML, linux-fbdev@vger.kernel.org,
Network Development
In-Reply-To: <848b3f21-9516-8a66-e4b3-9056ce38d6f6@roeck-us.net>
On Tue, Jul 11, 2017 at 8:10 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>
> The hwmon warnings are all about supporting no more than 9,999 sensors
> (applesmc) to 999,999,999 sensors (scpi) of a given type.
Yeah, I think that's enough.
> Easy "fix" would be to replace snprintf() with scnprintf(), presumably
> because gcc doesn't know about scnprintf().
If that's the case, I'd prefer just turning off the format-truncation
(but not overflow) warning with '-Wno-format-trunction".
But maybe we can at least start it on a subsystem-by-subsystem basis
after people have verified their own subsusystem?
Linus
^ permalink raw reply
* Re: Lots of new warnings with gcc-7.1.1
From: Linus Torvalds @ 2017-07-12 3:41 UTC (permalink / raw)
To: Guenter Roeck
Cc: Tejun Heo, Jean Delvare, Bartlomiej Zolnierkiewicz,
Sathya Prakash, James E.J. Bottomley, Greg Kroah-Hartman,
the arch/x86 maintainers, xen-devel, linux-block,
Linux Media Mailing List, IDE-ML, linux-fbdev@vger.kernel.org,
Network Development
In-Reply-To: <CA+55aFyKpezj3oHwtBShyf9x-DJNAGQhrq55iVGM42eWKQtP3w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1132 bytes --]
On Tue, Jul 11, 2017 at 8:17 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> If that's the case, I'd prefer just turning off the format-truncation
> (but not overflow) warning with '-Wno-format-trunction".
Doing
KBUILD_CFLAGS += $(call cc-disable-warning, format-truncation)
in the main Makefile certainly cuts down on the warnings.
We still have some overflow warnings, including the crazy one where
gcc doesn't see that the number of max7315 boards is very limited.
But those could easily be converted to just snprintf() instead, and
then the truncation warning disabling takes care of it. Maybe that's
the right answer.
We also have about a bazillion
warning: ‘*’ in boolean context, suggest ‘&&’ instead
warnings in drivers/ata/libata-core.c, all due to a single macro that
uses a pattern that gcc-7.1.1 doesn't like. The warning looks a bit
debatable, but I suspect the macro could easily be changed too.
Tejun, would you hate just moving the "multiply by 1000" part _into_
that EZ() macro? Something like the attached (UNTESTED!) patch?
Linus
[-- Attachment #2: patch.diff --]
[-- Type: text/plain, Size: 1404 bytes --]
drivers/ata/libata-core.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 8453f9a4682f..4c7d5a138495 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3231,19 +3231,19 @@ static const struct ata_timing ata_timing[] = {
};
#define ENOUGH(v, unit) (((v)-1)/(unit)+1)
-#define EZ(v, unit) ((v)?ENOUGH(v, unit):0)
+#define EZ(v, unit) ((v)?ENOUGH((v)*1000, unit):0)
static void ata_timing_quantize(const struct ata_timing *t, struct ata_timing *q, int T, int UT)
{
- q->setup = EZ(t->setup * 1000, T);
- q->act8b = EZ(t->act8b * 1000, T);
- q->rec8b = EZ(t->rec8b * 1000, T);
- q->cyc8b = EZ(t->cyc8b * 1000, T);
- q->active = EZ(t->active * 1000, T);
- q->recover = EZ(t->recover * 1000, T);
- q->dmack_hold = EZ(t->dmack_hold * 1000, T);
- q->cycle = EZ(t->cycle * 1000, T);
- q->udma = EZ(t->udma * 1000, UT);
+ q->setup = EZ(t->setup, T);
+ q->act8b = EZ(t->act8b, T);
+ q->rec8b = EZ(t->rec8b, T);
+ q->cyc8b = EZ(t->cyc8b, T);
+ q->active = EZ(t->active, T);
+ q->recover = EZ(t->recover, T);
+ q->dmack_hold = EZ(t->dmack_hold, T);
+ q->cycle = EZ(t->cycle, T);
+ q->udma = EZ(t->udma, UT);
}
void ata_timing_merge(const struct ata_timing *a, const struct ata_timing *b,
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox