When ata_qc_complete() schedules a command for EH using
ata_qc_schedule_eh(), blk_abort_request() will be called, which leads to
req->q->mq_ops->timeout() / scsi_timeout() being called.
scsi_timeout(), if the LLDD has no abort handler (libata has no abort
handler), will set host byte to DID_TIME_OUT, and then call
scsi_eh_scmd_add() to add the command to EH.
Thus, when commands first enter libata's EH strategy_handler, all the
commands that have been added to EH will have DID_TIME_OUT set.
Commit e5dd410acb ("ata: libata: Clear DID_TIME_OUT for ATA PT commands
with sense data") clears this bogus DID_TIME_OUT flag for all commands
that reached libata's EH strategy_handler.
libata has its own flag (AC_ERR_TIMEOUT), that it sets for commands that
have not received a completion at the time of entering EH.
ata_eh_worth_retry() has no special handling for AC_ERR_TIMEOUT, so by
default timed out commands will get flag ATA_QCFLAG_RETRY set, and will be
retried after the port has been reset (ata_eh_link_autopsy() always
triggers a port reset if any command has AC_ERR_TIMEOUT set).
For a command that has ATA_QCFLAG_RETRY set, while also having an error
flag set (e.g. AC_ERR_TIMEOUT), ata_eh_finish() will not increment
scmd->allowed, so the command will at most be retried scmd->allowed number
of times (which by default is set to 3).
However, scsi_eh_flush_done_q() will only retry commands for which
scsi_noretry_cmd() returns false.
For a command that has DID_TIME_OUT set, while also having either the
FAILFAST flag set, or the command being a passthrough command,
scsi_noretry_cmd() will return true. Thus, such a command will never be
retried.
Thus, make sure that libata sets SCSI's DID_TIME_OUT flag for commands that
actually timed out (libata's AC_ERR_TIMEOUT flag), such that timed out
commands will once again not be retried if they are also a FAILFAST or
passthrough command.
Cc: stable@vger.kernel.org
Fixes: e5dd410acb ("ata: libata: Clear DID_TIME_OUT for ATA PT commands with sense data")
Reported-by: Lai, Yi <yi1.lai@linux.intel.com>
Closes: https://lore.kernel.org/linux-ide/ZxYz871I3Blsi30F@ly-workstation/
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Link: https://lore.kernel.org/r/20241023105540.1070012-2-cassel@kernel.org
Signed-off-by: Niklas Cassel <cassel@kernel.org>
A user reported that commit aa3998dbeb ("ata: libata-scsi: Disable scsi
device manage_system_start_stop") introduced a spin down + immediate spin
up of the disk both when entering and when resuming from hibernation.
This behavior was not there before, and causes an increased latency both
when entering and when resuming from hibernation.
Hibernation is done by three consecutive PM events, in the following order:
1) PM_EVENT_FREEZE
2) PM_EVENT_THAW
3) PM_EVENT_HIBERNATE
Commit aa3998dbeb ("ata: libata-scsi: Disable scsi device
manage_system_start_stop") modified ata_eh_handle_port_suspend() to call
ata_dev_power_set_standby() (which spins down the disk), for both event
PM_EVENT_FREEZE and event PM_EVENT_HIBERNATE.
Documentation/driver-api/pm/devices.rst, section "Entering Hibernation",
explicitly mentions that PM_EVENT_FREEZE does not have to be put the device
in a low-power state, and actually recommends not doing so. Thus, let's not
spin down the disk on PM_EVENT_FREEZE. (The disk will instead be spun down
during the subsequent PM_EVENT_HIBERNATE event.)
This way, PM_EVENT_FREEZE will behave as it did before commit aa3998dbeb
("ata: libata-scsi: Disable scsi device manage_system_start_stop"), while
PM_EVENT_HIBERNATE will continue to spin down the disk.
This will avoid the superfluous spin down + spin up when entering and
resuming from hibernation, while still making sure that the disk is spun
down before actually entering hibernation.
Cc: stable@vger.kernel.org # v6.6+
Fixes: aa3998dbeb ("ata: libata-scsi: Disable scsi device manage_system_start_stop")
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Link: https://lore.kernel.org/r/20241008135843.1266244-2-cassel@kernel.org
Signed-off-by: Niklas Cassel <cassel@kernel.org>
asm/unaligned.h is always an include of asm-generic/unaligned.h;
might as well move that thing to linux/unaligned.h and include
that - there's nothing arch-specific in that header.
auto-generated by the following:
for i in `git grep -l -w asm/unaligned.h`; do
sed -i -e "s/asm\/unaligned.h/linux\/unaligned.h/" $i
done
for i in `git grep -l -w asm-generic/unaligned.h`; do
sed -i -e "s/asm-generic\/unaligned.h/linux\/unaligned.h/" $i
done
git mv include/asm-generic/unaligned.h include/linux/unaligned.h
git mv tools/include/asm-generic/unaligned.h tools/include/linux/unaligned.h
sed -i -e "/unaligned.h/d" include/asm-generic/Kbuild
sed -i -e "s/__ASM_GENERIC/__LINUX/" include/linux/unaligned.h tools/include/linux/unaligned.h
- Fix a NULL pointer dereference introduced by the recent cleanups of
the command duration limits feature handling (from me)
- Fix incorrect generation of the mode sense data for the
ALL_SUB_MPAGES page (from me)
-----BEGIN PGP SIGNATURE-----
iHUEABYKAB0WIQSRPv8tYSvhwAzJdzjdoc3SxdoYdgUCZvajtgAKCRDdoc3SxdoY
dsmNAPsF4O+6lri5SVPasNa0d96AOKdqdHI3bsqMo2tEWZNIswEA78F15giSxZOe
uEJyleGRhZ5PZIh0aY8MLrfAxJejFgI=
=HF/z
-----END PGP SIGNATURE-----
Merge tag 'ata-6.12-rc1-part2' of git://git.kernel.org/pub/scm/linux/kernel/git/libata/linux
Pull ata fixes from Damien Le Moal:
- Fix a NULL pointer dereference introduced by the recent cleanups of
the command duration limits feature handling (me)
- Fix incorrect generation of the mode sense data for the
ALL_SUB_MPAGES page (me)
* tag 'ata-6.12-rc1-part2' of git://git.kernel.org/pub/scm/linux/kernel/git/libata/linux:
ata: libata-scsi: Fix ata_msense_control() CDL page reporting
ata: libata-scsi: Fix ata_msense_control_spgt2()
This concludes a long journey towards replacing the old
board files with devictree description on the Cirrus Logic
EP93xx platform.
Nikita Shubin has been working on this for a long time,
for details see the last post on
https://lore.kernel.org/lkml/20240909-ep93xx-v12-0-e86ab2423d4b@maquefel.me/
-----BEGIN PGP SIGNATURE-----
iQIzBAABCgAdFiEEiK/NIGsWEZVxh/FrYKtH/8kJUicFAmb1croACgkQYKtH/8kJ
UicY0g//XXEXcBgE2CLfKzGimN3gREIElEqFCpd7v32XWGIQNFdS7StiGqNx1MeU
UYdILm97ldgpx+NnHd3Cb9HbLQ1CTIIvAZ2ngFLDeeZO+wgzBVxWTrdUUp57ZIBn
5Fq0hNaR1bfqSr+J+ZbgizH5N96EvLr3OPz/eJetY7egVBUID/0OpwssPJxW1Ns0
f+W+yIc7BomVa71xGgI+RkHrG/5DSaoFtrB+ESt7q1nNUIeMn32JqBYqE0U2iCRN
ADO8I+WfAjIcO1uN5n3KM3tigZI3GKSrBdllByr8wWNbp9l5rMYfFAPEaI109iyI
7PFrB6qhAlY9LckXMNhwLyjlnWt6qrI0B+tyg+3tW6+4OwFnpPN0cIhszFPOmrhv
njsDSvybp0q9V6Mn7f394H6v9sk9RHr68mpu12hO65UBP7Qe7mrdl3snnFcm0FHL
jCLnvjdmCSqRlV6YFsKDHuDzZOG88sAwH0mySKd3c/CVvgaNDsaJduelPGpuXlXX
P7op6D8kyKFKfmwK0kz3t+3+2ozgYq3nu4amI7rJ72MOvJKBocTwwqpAesIuegde
bn3ZN30yZDTbfEFuveOAzx7rqDlZYX/tN0uspL4VBN0rdayxBng5hneV2PypTtW0
wE9ptz5qIz8AssJ7NInwpgRTDjEut4SY3m3CS2/66V08B4EznAA=
=Y3Cd
-----END PGP SIGNATURE-----
Merge tag 'soc-ep93xx-dt-6.12' of git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc
Pull SoC update from Arnd Bergmann:
"Convert ep93xx to devicetree
This concludes a long journey towards replacing the old board files
with devictree description on the Cirrus Logic EP93xx platform.
Nikita Shubin has been working on this for a long time, for details
see the last post on
https://lore.kernel.org/lkml/20240909-ep93xx-v12-0-e86ab2423d4b@maquefel.me/"
* tag 'soc-ep93xx-dt-6.12' of git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc: (47 commits)
dt-bindings: gpio: ep9301: Add missing "#interrupt-cells" to examples
MAINTAINERS: Update EP93XX ARM ARCHITECTURE maintainer
soc: ep93xx: drop reference to removed EP93XX_SOC_COMMON config
net: cirrus: use u8 for addr to calm down sparse
dmaengine: cirrus: use snprintf() to calm down gcc 13.3.0
dmaengine: ep93xx: Fix a NULL vs IS_ERR() check in probe()
pinctrl: ep93xx: Fix raster pins typo
spi: ep93xx: update kerneldoc comments for ep93xx_spi
clk: ep93xx: Fix off by one in ep93xx_div_recalc_rate()
clk: ep93xx: add module license
dmaengine: cirrus: remove platform code
ASoC: cirrus: edb93xx: Delete driver
ARM: ep93xx: soc: drop defines
ARM: ep93xx: delete all boardfiles
ata: pata_ep93xx: remove legacy pinctrl use
pwm: ep93xx: drop legacy pinctrl
ARM: ep93xx: DT for the Cirrus ep93xx SoC platforms
ARM: dts: ep93xx: Add EDB9302 DT
ARM: dts: ep93xx: add ts7250 board
ARM: dts: add Cirrus EP93XX SoC .dtsi
...
When the user requests the ALL_SUB_MPAGES mode sense page,
ata_msense_control() adds the CDL_T2A_SUB_MPAGE twice instead of adding
the CDL_T2A_SUB_MPAGE and CDL_T2B_SUB_MPAGE pages information. Correct
the second call to ata_msense_control_spgt2() to report the
CDL_T2B_SUB_MPAGE page.
Fixes: 673b2fe6ff ("scsi: ata: libata-scsi: Add support for CDL pages mode sense")
Cc: stable@vger.kernel.org
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
ata_msense_control_spgt2() can be called even for devices that do not
support CDL when the user requests the ALL_SUB_MPAGES mode sense page,
but for such device, this will cause a NULL pointer dereference as
dev->cdl is NULL. Similarly, we should not return any data if
ata_msense_control_spgt2() is called when the user requested the
CDL_T2A_SUB_MPAGE or CDL_T2B_SUB_MPAGE pages for a device that does not
support CDL.
Avoid this potential NULL pointer dereference by checking if the device
support CDL on entry to ata_msense_control_spgt2() and return 0 if it
does not support CDL.
Reported-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
Tested-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
Fixes: 602bcf2126 ("ata: libata: Improve CDL resource management")
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
- Convert the qcom AHCI controller DT bindings to DT schema (from
Rayyan)
- Cleanup of libata core and drivers code handling controller and
device quirks to rename "blacklist" to the more neutral "quirk" and
to replace the rarely used "horkage" term with the more common
"quirk" naming (from me)
- Add libata-core message to print the quirks applied to a controller
or device (from me)
- Remove the not-so-useful function ata_noop_qc_prep() from libata core
(from me)
- ahci_imx driver cleanup, improvements and DT bindings compatible
strings update (from Richard and Dan)
- libahci_platform improvements (from Zhang)
- Remove obsolete functions declarations from libata header files (from
Gaosheng)
- Improve teh ahci_brcm driver using managed device resources funetions
(from Zhang)
- Introduce new helper function to improve libata EH code readability
(from Niklas)
- Enable module autoloading for the pata_ftide010, pata_ixp4xx and
sata_gemini drivers (from Liao)
- Move SATA related functions and data declaraions from libata-core to
libata-sata (from me)
- Rename the function handling the sense data for successful NCQ
commands log to better reflect that function actions (from me)
- Reduce libata memory usage by moving port resources to struct
ata_device and by optimizing the management of resources for CDL
capable devices (from me)
- Improve libata-eh handling of failed ATA passthrough commands (from
Niklas)
-----BEGIN PGP SIGNATURE-----
iHUEABYKAB0WIQSRPv8tYSvhwAzJdzjdoc3SxdoYdgUCZuvJ2wAKCRDdoc3SxdoY
dhVhAP9Zctzd9Ylb17eO7vVDwA0p7nzqyDxwxWPL8p3FtSm8PQD+PWQaLAPfWmKD
X8RqWv9WDGvCGUTEBBSjiZslEp7ccwk=
=68kF
-----END PGP SIGNATURE-----
Merge tag 'ata-6.12-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/libata/linux
Pull ata updates from Damien Le Moal:
- Convert the qcom AHCI controller DT bindings to DT schema (from
Rayyan)
- Cleanup of libata core and drivers code handling controller and
device quirks to rename "blacklist" to the more neutral "quirk" and
to replace the rarely used "horkage" term with the more common
"quirk" naming (me)
- Add libata-core message to print the quirks applied to a controller
or device (me)
- Remove the not-so-useful function ata_noop_qc_prep() from libata core
(me)
- ahci_imx driver cleanup, improvements and DT bindings compatible
strings update (Richard and Dan)
- libahci_platform improvements (Zhang)
- Remove obsolete functions declarations from libata header files (from
Gaosheng)
- Improve teh ahci_brcm driver using managed device resources funetions
(Zhang)
- Introduce new helper function to improve libata EH code readability
(Niklas)
- Enable module autoloading for the pata_ftide010, pata_ixp4xx and
sata_gemini drivers (Liao)
- Move SATA related functions and data declaraions from libata-core to
libata-sata (me)
- Rename the function handling the sense data for successful NCQ
commands log to better reflect that function actions (me)
- Reduce libata memory usage by moving port resources to struct
ata_device and by optimizing the management of resources for CDL
capable devices (me)
- Improve libata-eh handling of failed ATA passthrough commands
(Niklas)
* tag 'ata-6.12-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/libata/linux: (39 commits)
ata: libata: Clear DID_TIME_OUT for ATA PT commands with sense data
ata: libata: Fix W=1 compilation warning
ata: libata: Improve CDL resource management
ata: libata: Introduce ata_dev_free_resources
ata: libata: Move sector_buf from struct ata_port to struct ata_device
ata: libata: Rename ata_eh_read_sense_success_ncq_log()
ata: libata: Move sata_std_hardreset() definition to libata-sata.c
ata: libata: Move sata_down_spd_limit() to libata-sata.c
ata: libata: Improve __ata_qc_complete()
ata: libata-scsi: Improve ata_scsi_handle_link_detach()
ata: libata: Cleanup libata-transport
ata: sata_gemini: Enable module autoloading
ata: pata_ixp4xx: Enable module autoloading
ata: pata_ftide010: Enable module autoloading
ata: libata: Add helper ata_eh_decide_disposition()
ata: ahci_brcm: Use devm_platform_ioremap_resource_byname() helper function
ata: libata: Remove obsolete function declarations
ata: ahci_imx: Fix error code in probe()
ata: libahci_platform: Simplify code with for_each_child_of_node_scoped()
ata: ahci_imx: Correct the email address
...
Drop legacy acquire/release since we are using pinctrl for this now.
Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
Tested-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Acked-by: Damien Le Moal <dlemoal@kernel.org>
Acked-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
- add OF ID match table
- drop platform DMA and filters
- change DMA setup to OF, so we can defer probe
Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
Tested-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Acked-by: Damien Le Moal <dlemoal@kernel.org>
Acked-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
When ata_qc_complete() schedules a command for EH using
ata_qc_schedule_eh(), blk_abort_request() will be called, which leads to
req->q->mq_ops->timeout() / scsi_timeout() being called.
scsi_timeout(), if the LLDD has no abort handler (libata has no abort
handler), will set host byte to DID_TIME_OUT, and then call
scsi_eh_scmd_add() to add the command to EH.
Thus, when commands first enter libata's EH strategy_handler, all the
commands that have been added to EH will have DID_TIME_OUT set.
libata has its own flag (AC_ERR_TIMEOUT), that it sets for commands that
have not received a completion at the time of entering EH.
Thus, libata doesn't really care about DID_TIME_OUT at all, and currently
clears the host byte at the end of EH, in ata_scsi_qc_complete(), before
scsi_eh_finish_cmd() is called.
However, this clearing in ata_scsi_qc_complete() is currently only done
for commands that are not ATA passthrough commands.
Since the host byte is visible in the completion that we return to user
space for ATA passthrough commands, for ATA passthrough commands that got
completed via EH (commands with sense data), the user will incorrectly see:
ATA pass-through(16): transport error: Host_status=0x03 [DID_TIME_OUT]
Fix this by moving the clearing of the host byte (which is currently only
done for commands that are not ATA passthrough commands) from
ata_scsi_qc_complete() to the start of EH (regardless if the command is
ATA passthrough or not).
While at it, use the proper helper function to clear the host byte, rather
than open coding the clearing.
This will make sure that we:
-Correctly clear DID_TIME_OUT for both ATA passthrough commands and
commands that are not ATA passthrough commands.
-Do not needlessly clear the host byte for commands that did not go via EH.
ata_scsi_qc_complete() is called both for commands that are completed
normally (without going via EH), and for commands that went via EH,
however, only commands that went via EH will have DID_TIME_OUT set.
Fixes: 24aeebbf8e ("scsi: ata: libata: Change ata_eh_request_sense() to not set CHECK_CONDITION")
Reported-by: Igor Pylypiv <ipylypiv@google.com>
Closes: https://lore.kernel.org/linux-ide/ZttIN8He8TOZ7Lct@google.com/
Signed-off-by: Niklas Cassel <cassel@kernel.org>
Tested-by: Igor Pylypiv <ipylypiv@google.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Commit c494708d1f ("ata: libata: Cleanup libata-transport")
inadvertently changed the name of the link argument to ata_link in the
kdoc description of ata_tlink_add(), causing warnings to be issue when
compiling with W=1:
drivers/ata/libata-transport.c:690: warning: Function parameter or
struct member 'link' not described in 'ata_tlink_add'
drivers/ata/libata-transport.c:690: warning: Excess function parameter
'ata_link' description in 'ata_tlink_add'
Change the kdoc argument name to "link" to avoid these warnings.
Fixes: c494708d1f ("ata: libata: Cleanup libata-transport")
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
The ncq_sense_buf buffer field of struct ata_port is allocated and used
only for devices that support the Command Duration Limits (CDL) feature.
However, the cdl buffer of struct ata_device, which is used to cache the
command duration limits log page for devices supporting CDL is always
allocated as part of struct ata_device, which is wasteful of memory for
devices that do not support this feature.
Clean this up by defining both buffers as part of the new ata_cdl
structure and allocating this structure only for devices that support
the CDL feature. This new structure is attached to struct ata_device
using the cdl pointer.
The functions ata_dev_init_cdl_resources() and
ata_dev_cleanup_cdl_resources() are defined to manage this new structure
allocation, initialization and freeing when a port is removed or a
device disabled. ata_dev_init_cdl_resources() is called from
ata_dev_config_cdl() only for devices that support CDL.
ata_dev_cleanup_cdl_resources() is called from ata_dev_free_resources()
to free the ata_cdl structure when a device is being disabled by EH.
Note that the name of the former cdl log buffer of struct ata_device is
changed to desc_log_buf to make it clearer that it is a buffer for the
limit descriptors log page.
This change reduces the size of struct ata_device, thus reducing memory
usage for ATA devices that do not support the CDL feature.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Introduce the function ata_dev_free_resources() to free the resources
allocated to support a device features. For now, this function is
reduced to calling zpodd_exit() for devices that have this feature
enabled.
ata_dev_free_resources() is called from ata_eh_dev_disable() as this
function is always called for all devices attached to a port that is
being detached and for devices that are being disabled due to being
removed (detached) from the system or due to errors.
With this change, the call to zpodd_exit() done in ata_port_detach()
and ata_scsi_handle_link_detach() are removed as these functions
remove all devices attached to the link or port using libata EH, thus
resulting in ata_eh_dev_disable() being called and the zpodd_exit()
function being executed.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
The 512B buffer sector_buf field of struct ata_port is used for scanning
devices as well as during error recovery with ata EH. This buffer is
thus useless if a port does not have a device connected to it.
And also given that commands using this buffer are issued to devices,
and not to ports, move this buffer definition from struct ata_port to
struct ata_device.
This change slightly increases system memory usage for systems using a
port-multiplier as in that case we do not need a per-device buffer for
scanning devices (PMP does not allow parallel scanning) nor for EH (as
when entering EH we are guaranteed that all commands to all devices
connected to the PMP have completed or have been aborted). However,
this change reduces memory usage on systems that have many ports with
only few devices rives connected, which is a much more common use case
than the PMP use case.
Suggested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
The function ata_eh_read_sense_success_ncq_log() does more that just
reading the sense data for successful NCQ commands log page as it also
sets the sense data for all commands listed in the log page.
Rename this function to ata_eh_get_ncq_success_sense() to better
describe what the function does. Furthermore, since this function is
only called from ata_eh_get_success_sense() in libata-eh.c, there is no
need to export it and its declaration can be moved to
drivers/ata/libata.h.
To be consistent with this change, the function
ata_eh_read_sense_success_non_ncq() is also renamed to
ata_eh_get_non_ncq_success_sense().
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Unlike ata_std_prereset() and ata_std_postreset(), the function
sata_std_hardreset() applies only to SATA devices, as its name implies.
So move its definition to libata-sata.c.
Together with this, also move the definition of sata_port_ops to
libata-sata.c, where it belongs.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Move the definition of the function sata_down_spd_limit() to
libata-sata.c where it belongs, together with sata_set_spd().
The helper function ata_sstatus_online() is also changed to be an
inline function defined in drivers/ata/libata.h.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
The function __ata_qc_complete() is always called with a qc that already
has been dereferenced and so is guaranteed to be non-NULL (as otherwise
the kernel would have crashed). So remove the warning for a NULL qc as
it is useless.
Furthermore, the qc passed to __ata_qc_complete() must always be marked
as active with the ATA_QCFLAG_ACTIVE flag. If that is not the case, in
addition to the existing warning, return early so that we do not attempt
to complete an invalid qc.
Finally, fix the comment related to clearing the qc active flag as that
operation applies to all devices, not just ATAPI ones.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
A struct ata_device flags should always be set and cleared with the
device port locked. Testing for a flag should thus also be done while
holding the device port lock. In accordance to this principle, modify
ata_scsi_handle_link_detach() to test and clear the ATA_DFLAG_DETACHED
flag while holding the device port lock.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Move the ATA link transport device related functions after the ATA
transport device related functions to avoid the need for forward
declaring ata_tdev_add() and ata_tdev_delete().
And while at it, do the following:
1) Change ata_is_ata_dev() and ata_is_link() to return a boolean
2) Fix a pointer declaration style in ata_is_ata_dev()
3) Improve the kdoc comments for ata_tdev_free(), ata_tdev_delete(),
ata_tdev_add(), ata_tlink_delete() and ata_tlink_add()
No functional changes are introduced by this cleanup.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Add MODULE_DEVICE_TABLE(), so modules can be properly autoloaded based
on the alias from of_device_id table.
Signed-off-by: Liao Chen <liaochen4@huawei.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Add MODULE_DEVICE_TABLE(), so modules can be properly autoloaded based
on the alias from of_device_id table.
Signed-off-by: Liao Chen <liaochen4@huawei.com>
Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Add MODULE_DEVICE_TABLE(), so modules can be properly autoloaded based
on the alias from of_device_id table.
Signed-off-by: Liao Chen <liaochen4@huawei.com>
Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Every time I see libata code calling scsi_check_sense(), I get confused
why the code path that is working fine for SCSI code, is not sufficient
for libata code.
The reason is that SCSI usually gets the sense data as part of the
completion, and will thus automatically call scsi_check_sense(), which
will set the SCSI ML byte (if any).
However, for libata queued commands, we always need to fetch the sense
data via SCSI EH, and thus do not get the luxury of having
scsi_check_sense() called automatically.
Add a new helper, ata_eh_decide_disposition(), that has a ata_eh_ prefix
to more clearly highlight that this is only needed for code called by EH,
while also having a similar name to scsi_decide_disposition(), such that
it is easier to compare the libata code with the equivalent SCSI code.
Also add a big kdoc comment explaining why this helper is called/needed in
the first place.
Signed-off-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
platform_get_resource_byname() and devm_ioremap_resource() can be
replaced by devm_platform_ioremap_resource_byname(), which can
simplify the code logic a bit, No functional change here.
Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
The function ata_schedule_scsi_eh() was removed with commit
f8bbfc247e ("[PATCH] SCSI: make scsi_implement_eh() generic API for
SCSI transports"), and the function ata_sff_irq_clear() was removed
with commit 37f65b8bc262("libata-sff: ata_sff_irq_clear() is BMDMA
specific").
Remove the now useless declarations of these functions in
drivers/ata/libata.h and include/linux/libata.h.
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
In ata_host_alloc(), if devres_alloc() fails to allocate the device host
resource data pointer, the already allocated ata_host structure is not
freed before returning from the function. This results in a potential
memory leak.
Call kfree(host) before jumping to the error handling path to ensure
that the ata_host structure is properly freed if devres_alloc() fails.
Fixes: 2623c7a5f2 ("libata: add refcounting to ata_host")
Cc: stable@vger.kernel.org
Signed-off-by: Zheng Qixing <zhengqixing@huawei.com>
Reviewed-by: Yu Kuai <yukuai3@huawei.com>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
The overflow/underflow conditions in pata_macio_qc_prep() should never
happen. But if they do there's no need to kill the system entirely, a
WARN and failing the IO request should be sufficient and might allow the
system to keep running.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Kolbjørn and Jonáš reported that their 32-bit PowerMacs were crashing
in pata-macio since commit 09fe2bfa6b ("ata: pata_macio: Fix
max_segment_size with PAGE_SIZE == 64K").
For example:
kernel BUG at drivers/ata/pata_macio.c:544!
Oops: Exception in kernel mode, sig: 5 [#1]
BE PAGE_SIZE=4K MMU=Hash SMP NR_CPUS=2 DEBUG_PAGEALLOC PowerMac
...
NIP pata_macio_qc_prep+0xf4/0x190
LR pata_macio_qc_prep+0xfc/0x190
Call Trace:
0xc1421660 (unreliable)
ata_qc_issue+0x14c/0x2d4
__ata_scsi_queuecmd+0x200/0x53c
ata_scsi_queuecmd+0x50/0xe0
scsi_queue_rq+0x788/0xb1c
__blk_mq_issue_directly+0x58/0xf4
blk_mq_plug_issue_direct+0x8c/0x1b4
blk_mq_flush_plug_list.part.0+0x584/0x5e0
__blk_flush_plug+0xf8/0x194
__submit_bio+0x1b8/0x2e0
submit_bio_noacct_nocheck+0x230/0x304
btrfs_work_helper+0x200/0x338
process_one_work+0x1a8/0x338
worker_thread+0x364/0x4c0
kthread+0x100/0x104
start_kernel_thread+0x10/0x14
That commit increased max_segment_size to 64KB, with the justification
that the SCSI core was already using that size when PAGE_SIZE == 64KB,
and that there was existing logic to split over-sized requests.
However with a sufficiently large request, the splitting logic causes
each sg to be split into two commands in the DMA table, leading to
overflow of the DMA table, triggering the BUG_ON().
With default settings the bug doesn't trigger, because the request size
is limited by max_sectors_kb == 1280, however max_sectors_kb can be
increased, and apparently some distros do that by default using udev
rules.
Fix the bug for 4KB kernels by reverting to the old max_segment_size.
For 64KB kernels the sg_tablesize needs to be halved, to allow for the
possibility that each sg will be split into two.
Fixes: 09fe2bfa6b ("ata: pata_macio: Fix max_segment_size with PAGE_SIZE == 64K")
Cc: stable@vger.kernel.org # v6.10+
Reported-by: Kolbjørn Barmen <linux-ppc@kolla.no>
Closes: https://lore.kernel.org/all/62d248bb-e97a-25d2-bcf2-9160c518cae5@kolla.no/
Reported-by: Jonáš Vidra <vidra@ufal.mff.cuni.cz>
Closes: https://lore.kernel.org/all/3b6441b8-06e6-45da-9e55-f92f2c86933e@ufal.mff.cuni.cz/
Tested-by: Kolbjørn Barmen <linux-ppc@kolla.no>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
for_each_child_of_node_scoped() can put the device_node automatically.
So let's use it to make the code simpler by avoiding the need to
explicitly call of_node_put().
Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com>
Link: https://lore.kernel.org/r/20240810023426.110624-1-zhangzekun11@huawei.com
Signed-off-by: Niklas Cassel <cassel@kernel.org>
This reverts commit 28ab976911.
Sense data can be in either fixed format or descriptor format.
SAT-6 revision 1, "10.4.6 Control mode page", defines the D_SENSE bit:
"The SATL shall support this bit as defined in SPC-5 with the following
exception: if the D_ SENSE bit is set to zero (i.e., fixed format sense
data), then the SATL should return fixed format sense data for ATA
PASS-THROUGH commands."
The libata SATL has always kept D_SENSE set to zero by default. (It is
however possible to change the value using a MODE SELECT SG_IO command.)
Failed ATA PASS-THROUGH commands correctly respected the D_SENSE bit,
however, successful ATA PASS-THROUGH commands incorrectly returned the
sense data in descriptor format (regardless of the D_SENSE bit).
Commit 28ab976911 ("ata: libata-scsi: Honor the D_SENSE bit for
CK_COND=1 and no error") fixed this bug for successful ATA PASS-THROUGH
commands.
However, after commit 28ab976911 ("ata: libata-scsi: Honor the D_SENSE
bit for CK_COND=1 and no error"), there were bug reports that hdparm,
hddtemp, and udisks were no longer working as expected.
These applications incorrectly assume the returned sense data is in
descriptor format, without even looking at the RESPONSE CODE field in the
returned sense data (to see which format the returned sense data is in).
Considering that there will be broken versions of these applications around
roughly forever, we are stuck with being bug compatible with older kernels.
Cc: stable@vger.kernel.org # 4.19+
Reported-by: Stephan Eisvogel <eisvogel@seitics.de>
Reported-by: Christian Heusel <christian@heusel.eu>
Closes: https://lore.kernel.org/linux-ide/0bf3f2f0-0fc6-4ba5-a420-c0874ef82d64@heusel.eu/
Fixes: 28ab976911 ("ata: libata-scsi: Honor the D_SENSE bit for CK_COND=1 and no error")
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Link: https://lore.kernel.org/r/20240813131900.1285842-2-cassel@kernel.org
Signed-off-by: Niklas Cassel <cassel@kernel.org>
The RXWM(RxWaterMark) sets the minimum number of free location within
the RX FIFO before the watermark is exceeded which in turn will cause
the Transport Layer to instruct the Link Layer to transmit HOLDS to the
transmitting end.
Based on the default RXWM value 0x20, RX FIFO overflow might be observed
on i.MX8QM MEK board, when some Gen3 SATA disks are used.
The FIFO overflow will result in CRC error, internal error and protocol
error, then the SATA link is not stable anymore.
To fix this issue, enlarge RX water mark setting from 0x20 to 0x29.
Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Link: https://lore.kernel.org/r/1723428055-27021-5-git-send-email-hongxing.zhu@nxp.com
Signed-off-by: Niklas Cassel <cassel@kernel.org>
i.MX8QM AHCI SATA doesn't need set AHB clock rate to config the vendor
specified TIMER1MS register.
Set AHB clock rate only for i.MX53 and i.MX6Q.
Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/1723428055-27021-4-git-send-email-hongxing.zhu@nxp.com
Signed-off-by: Niklas Cassel <cassel@kernel.org>
Clean up code by using PHY interface provided by the PHY driver under
PHY subsystem(drivers/phy/freescale/phy-fsl-imx8qm-hsio.c).
Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/1723428055-27021-3-git-send-email-hongxing.zhu@nxp.com
Signed-off-by: Niklas Cassel <cassel@kernel.org>
In ata_dev_print_quirks(), return early if ata_dev_print_info() returns
false or if we already printed quirk information. This is to avoid
printing a device quirks multiple times (that is, each time
ata_dev_revalidate() is called).
To remember if ata_dev_print_quirks() was already executed, define the
EH context flag ATA_EHI_DID_PRINT_QUIRKS and set this flag in
ata_dev_print_quirks().
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Fixes: 58157d607a ("ata: libata: Print quirks applied to devices")
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
The function ata_noop_qc_prep(), as its name implies, does nothing and
simply returns AC_ERR_OK. For drivers that do not need any special
preparations of queued commands, we can avoid having to define struct
ata_port qc_prep operation by simply testing if that operation is
defined or not in ata_qc_issue(). Make this change and remove
ata_noop_qc_prep().
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: John Garry <john.g.garry@oracle.com>
Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Rename the function hpt_dma_blacklisted() to the more neutral
hpt_dma_broken().
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Igor Pylypiv <ipylypiv@google.com>
Rename the function hpt_dma_blacklisted() to the more neutral
hpt_dma_broken().
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Igor Pylypiv <ipylypiv@google.com>
Remove the use of the term "blacklist". What the comment using that term
refers to does not seem to exist at all anyway as the driver does not
have such list but rather only a list of compatible controllers.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Igor Pylypiv <ipylypiv@google.com>
Remove the comment using the term "blacklist" from piix_init_sidpr().
That comment is useless given that the function piix_no_sidpr() name is
clear about what is being checked.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Igor Pylypiv <ipylypiv@google.com>
Rename the array sil_blacklist to sil_quirks as this name is more
neutral and is also consistent with how this driver define quirks with
the SIL_QUIRK_XXX flags.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Igor Pylypiv <ipylypiv@google.com>
Rephrase the comment for the eMachines entry in the sysids array of
ahci_broken_suspend() to not use the term blacklist.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Igor Pylypiv <ipylypiv@google.com>
Let's not use the term blacklist in the function
serverworks_osb4_filter() documentation comment and rather simply refer
to what that function looks at: the list of devices with groken UDMA5.
While at it, also constify the values of the csb_bad_ata100 array.
Of note is that all of this should probably be handled using libata
quirk mechanism but it is unclear if these UDMA5 quirks are specific
to this controller only.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Igor Pylypiv <ipylypiv@google.com>
Introduce the function ata_dev_print_quirks() to print the quirk flags
that will be applied to a scanned device. This new function is called
from ata_dev_quirks() when a match on a device model or device model
and revision is found for a device in the __ata_dev_quirks array.
To implement this function, the ATA_QUIRK_ flags are redefined using
the new enum ata_quirk which defines the bit shift for each quirk
flag. The array of strings ata_quirk_names is used to define the name
of each flag, which are printed by ata_dev_print_quirks().
Example output for a device listed in the __ata_dev_quirks array and
which has the ATA_QUIRK_DISABLE flag applied:
[10193.461270] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
[10193.469190] ata1.00: Model 'ASMT109x- Config', rev '2143 5', applying quirks: disable
[10193.469195] ata1.00: unsupported device, disabling
[10193.481564] ata1.00: disable device
enum ata_quirk also defines the __ATA_QUIRK_MAX value as one plus the
last quirk flag defined. This value is used in ata_dev_quirks() to add a
build time check that all quirk flags fit within the unsigned int
(32-bits) quirks field of struct ata_device.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Igor Pylypiv <ipylypiv@google.com>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
According to Wiktionary, the verb "hork" is computing slang defined as
"To foul up; to be occupied with difficulty, tangle, or unpleasantness;
to be broken" (https://en.wiktionary.org/wiki/hork#Verb). libata uses
this with the term "horkage" to refer to broken device features. Given
that this term is not widely used and its meaning unknown to many,
rename it to the more commonly used term "quirk", similar to many other
places in the kernel.
The renaming done is:
1) Rename all ATA_HORKAGE_XXX flags to ATA_QUIRK_XXX
2) Rename struct ata_device horkage field to quirks
3) Rename struct ata_blacklist_entry to struct ata_dev_quirks_entry. The
array of these structures defining quirks for known devices is
renamed __ata_dev_quirks.
4) The functions ata_dev_blacklisted() and ata_force_horkage() are
renamed to ata_dev_quirks() and ata_force_quirks() respectively.
5) All the force_horkage_xxx() macros are renamed to force_quirk_xxx()
And while at it, make sure that the type "unsigned int" is used
consistantly for quirk flags variables and data structure fields.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Igor Pylypiv <ipylypiv@google.com>
Rename the function ata_dma_blacklisted() to ata_dev_nodma() as this new
name is more neutral. The function signature is also changed to return a
boolean instead of an int.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Igor Pylypiv <ipylypiv@google.com>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Change the function ata_dev_knobble() to return a boolean instead of a
u8.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Igor Pylypiv <ipylypiv@google.com>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
