In order to simplify the clock-related code there is a way to convert the
current fixed clocks array into using the common bulk clocks kernel API
with dynamic set of the clock handlers and device-managed clock-resource
tracking. It's a bit tricky due to the complication coming from the
requirement to support the platforms (da850, spear13xx) with the
non-OF-based clock source, but still doable.
Before this modification there are two methods have been used to get the
clocks connected to an AHCI device: clk_get() - to get the very first
clock in the list and of_clk_get() - to get the rest of them. Basically
the platforms with non-OF-based clocks definition could specify only a
single reference clock source. The platforms with OF-hw clocks have been
luckier and could setup up to AHCI_MAX_CLKS clocks. Such semantic can be
retained with using devm_clk_bulk_get_all() to retrieve the clocks defined
via the DT firmware and devm_clk_get_optional() otherwise. In both cases
using the device-managed version of the methods will cause the automatic
resources deallocation on the AHCI device removal event. The only
complicated part in the suggested approach is the explicit allocation and
initialization of the clk_bulk_data structure instance for the non-OF
reference clocks. It's required in order to use the Bulk Clocks API for
the both denoted cases of the clocks definition.
Note aside with the clock-related code reduction and natural
simplification, there are several bonuses the suggested modification
provides. First of all the limitation of having no greater than
AHCI_MAX_CLKS clocks is now removed, since the devm_clk_bulk_get_all()
method will allocate as many reference clocks data descriptors as there
are clocks specified for the device. Secondly the clock names are
auto-detected. So the LLDD (glue) drivers can make sure that the required
clocks are specified just by checking the clock IDs in the clk_bulk_data
array. Thirdly using the handy Bulk Clocks kernel API improves the
clocks-handling code readability. And the last but not least this
modification implements a true optional clocks support to the
ahci_platform_get_resources() method. Indeed the previous clocks getting
procedure just stopped getting the clocks on any errors (aside from
non-critical -EPROBE_DEFER) in a way so the callee wasn't even informed
about abnormal loop termination. The new implementation lacks of such
problem. The ahci_platform_get_resources() will return an error code if
the corresponding clocks getting method ends execution abnormally.
Signed-off-by: Serge Semin <[email protected]>
---
Changelog v2:
- Convert to checking the error-case first in the devm_clk_bulk_get_all()
method invocation. (@Damien)
- Fix some grammar mistakes in the comments.
---
drivers/ata/ahci.h | 4 +-
drivers/ata/libahci_platform.c | 84 ++++++++++++++++------------------
2 files changed, 41 insertions(+), 47 deletions(-)
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index ad11a4c52fbe..c3770a19781b 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -38,7 +38,6 @@
enum {
AHCI_MAX_PORTS = 32,
- AHCI_MAX_CLKS = 5,
AHCI_MAX_SG = 168, /* hardware max is 64K */
AHCI_DMA_BOUNDARY = 0xffffffff,
AHCI_MAX_CMDS = 32,
@@ -339,7 +338,8 @@ struct ahci_host_priv {
u32 em_msg_type; /* EM message type */
u32 remapped_nvme; /* NVMe remapped device count */
bool got_runtime_pm; /* Did we do pm_runtime_get? */
- struct clk *clks[AHCI_MAX_CLKS]; /* Optional */
+ unsigned int n_clks;
+ struct clk_bulk_data *clks; /* Optional */
struct reset_control *rsts; /* Optional */
struct regulator **target_pwrs; /* Optional */
struct regulator *ahci_regulator;/* Optional */
diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index 8c9fbcc3043b..3cff86c225fd 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -8,6 +8,7 @@
* Anton Vorontsov <[email protected]>
*/
+#include <linux/clk-provider.h>
#include <linux/clk.h>
#include <linux/kernel.h>
#include <linux/gfp.h>
@@ -97,28 +98,14 @@ EXPORT_SYMBOL_GPL(ahci_platform_disable_phys);
* ahci_platform_enable_clks - Enable platform clocks
* @hpriv: host private area to store config values
*
- * This function enables all the clks found in hpriv->clks, starting at
- * index 0. If any clk fails to enable it disables all the clks already
- * enabled in reverse order, and then returns an error.
+ * This function enables all the clks found for the AHCI device.
*
* RETURNS:
* 0 on success otherwise a negative error code
*/
int ahci_platform_enable_clks(struct ahci_host_priv *hpriv)
{
- int c, rc;
-
- for (c = 0; c < AHCI_MAX_CLKS && hpriv->clks[c]; c++) {
- rc = clk_prepare_enable(hpriv->clks[c]);
- if (rc)
- goto disable_unprepare_clk;
- }
- return 0;
-
-disable_unprepare_clk:
- while (--c >= 0)
- clk_disable_unprepare(hpriv->clks[c]);
- return rc;
+ return clk_bulk_prepare_enable(hpriv->n_clks, hpriv->clks);
}
EXPORT_SYMBOL_GPL(ahci_platform_enable_clks);
@@ -126,16 +113,13 @@ EXPORT_SYMBOL_GPL(ahci_platform_enable_clks);
* ahci_platform_disable_clks - Disable platform clocks
* @hpriv: host private area to store config values
*
- * This function disables all the clks found in hpriv->clks, in reverse
- * order of ahci_platform_enable_clks (starting at the end of the array).
+ * This function disables all the clocks enabled before
+ * (bulk-clocks-disable function is supposed to do that in reverse
+ * from the enabling procedure order).
*/
void ahci_platform_disable_clks(struct ahci_host_priv *hpriv)
{
- int c;
-
- for (c = AHCI_MAX_CLKS - 1; c >= 0; c--)
- if (hpriv->clks[c])
- clk_disable_unprepare(hpriv->clks[c]);
+ clk_bulk_disable_unprepare(hpriv->n_clks, hpriv->clks);
}
EXPORT_SYMBOL_GPL(ahci_platform_disable_clks);
@@ -292,8 +276,6 @@ static void ahci_platform_put_resources(struct device *dev, void *res)
pm_runtime_disable(dev);
}
- for (c = 0; c < AHCI_MAX_CLKS && hpriv->clks[c]; c++)
- clk_put(hpriv->clks[c]);
/*
* The regulators are tied to child node device and not to the
* SATA device itself. So we can't use devm for automatically
@@ -374,8 +356,8 @@ static int ahci_platform_get_regulator(struct ahci_host_priv *hpriv, u32 port,
* 1) mmio registers (IORESOURCE_MEM 0, mandatory)
* 2) regulator for controlling the targets power (optional)
* regulator for controlling the AHCI controller (optional)
- * 3) 0 - AHCI_MAX_CLKS clocks, as specified in the devs devicetree node,
- * or for non devicetree enabled platforms a single clock
+ * 3) all clocks specified in the devicetree node, or a single
+ * clock for non-OF platforms (optional)
* 4) resets, if flags has AHCI_PLATFORM_GET_RESETS (optional)
* 5) phys (optional)
*
@@ -385,11 +367,10 @@ static int ahci_platform_get_regulator(struct ahci_host_priv *hpriv, u32 port,
struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
unsigned int flags)
{
+ int rc, child_nodes, enabled_ports = 0;
struct device *dev = &pdev->dev;
struct ahci_host_priv *hpriv;
- struct clk *clk;
struct device_node *child;
- int i, enabled_ports = 0, rc, child_nodes;
u32 mask_port_map = 0;
if (!devres_open_group(dev, NULL, GFP_KERNEL))
@@ -417,25 +398,38 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
goto err_out;
}
- for (i = 0; i < AHCI_MAX_CLKS; i++) {
+ /*
+ * Bulk clocks getting procedure can fail to find any clock due to
+ * running on a non-OF platform or due to the clocks being defined in
+ * bypass of the DT firmware (like da850, spear13xx). In that case we
+ * fallback to getting a single clock source right from the dev clocks
+ * list.
+ */
+ rc = devm_clk_bulk_get_all(dev, &hpriv->clks);
+ if (rc < 0)
+ goto err_out;
+
+ if (rc > 0) {
+ /* Got clocks in bulk */
+ hpriv->n_clks = rc;
+ } else {
/*
- * For now we must use clk_get(dev, NULL) for the first clock,
- * because some platforms (da850, spear13xx) are not yet
- * converted to use devicetree for clocks. For new platforms
- * this is equivalent to of_clk_get(dev->of_node, 0).
+ * No clock bulk found: fallback to manually getting
+ * the optional clock.
*/
- if (i == 0)
- clk = clk_get(dev, NULL);
- else
- clk = of_clk_get(dev->of_node, i);
-
- if (IS_ERR(clk)) {
- rc = PTR_ERR(clk);
- if (rc == -EPROBE_DEFER)
- goto err_out;
- break;
+ hpriv->clks = devm_kzalloc(dev, sizeof(*hpriv->clks), GFP_KERNEL);
+ if (!hpriv->clks) {
+ rc = -ENOMEM;
+ goto err_out;
+ }
+ hpriv->clks->clk = devm_clk_get_optional(dev, NULL);
+ if (IS_ERR(hpriv->clks->clk)) {
+ rc = PTR_ERR(hpriv->clks->clk);
+ goto err_out;
+ } else if (hpriv->clks->clk) {
+ hpriv->clks->id = __clk_get_name(hpriv->clks->clk);
+ hpriv->n_clks = 1;
}
- hpriv->clks[i] = clk;
}
hpriv->ahci_regulator = devm_regulator_get(dev, "ahci");
--
2.35.1
Hi Serge,
I love your patch! Yet something to improve:
[auto build test ERROR on axboe-block/for-next]
[also build test ERROR on robh/for-next linus/master v5.18-rc6 next-20220512]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Serge-Semin/ata-ahci-Add-DWC-Baikal-T1-AHCI-SATA-support/20220512-072125
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: sh-randconfig-r006-20220512 (https://download.01.org/0day-ci/archive/20220513/[email protected]/config)
compiler: sh4-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/71066bfbd7d3a8cab5b76032068f5bcdc6d99b21
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Serge-Semin/ata-ahci-Add-DWC-Baikal-T1-AHCI-SATA-support/20220512-072125
git checkout 71066bfbd7d3a8cab5b76032068f5bcdc6d99b21
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=sh SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
sh4-linux-ld: drivers/ata/libahci_platform.o: in function `ahci_platform_get_resources':
>> drivers/ata/libahci_platform.c:382: undefined reference to `__clk_get_name'
vim +382 drivers/ata/libahci_platform.c
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 347
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 348 /**
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 349 * ahci_platform_get_resources - Get platform resources
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 350 * @pdev: platform device to get resources for
16af2d65842d343 Kunihiko Hayashi 2018-08-22 351 * @flags: bitmap representing the resource to get
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 352 *
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 353 * This function allocates an ahci_host_priv struct, and gets the following
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 354 * resources, storing a reference to them inside the returned struct:
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 355 *
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 356 * 1) mmio registers (IORESOURCE_MEM 0, mandatory)
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 357 * 2) regulator for controlling the targets power (optional)
a37da9180f42c95 Corentin Labbe 2018-09-03 358 * regulator for controlling the AHCI controller (optional)
71066bfbd7d3a8c Serge Semin 2022-05-12 359 * 3) all clocks specified in the devicetree node, or a single
71066bfbd7d3a8c Serge Semin 2022-05-12 360 * clock for non-OF platforms (optional)
9d2ab9957397083 Kunihiko Hayashi 2018-08-22 361 * 4) resets, if flags has AHCI_PLATFORM_GET_RESETS (optional)
9d2ab9957397083 Kunihiko Hayashi 2018-08-22 362 * 5) phys (optional)
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 363 *
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 364 * RETURNS:
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 365 * The allocated ahci_host_priv on success, otherwise an ERR_PTR value
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 366 */
16af2d65842d343 Kunihiko Hayashi 2018-08-22 367 struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
16af2d65842d343 Kunihiko Hayashi 2018-08-22 368 unsigned int flags)
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 369 {
71066bfbd7d3a8c Serge Semin 2022-05-12 370 int rc, child_nodes, enabled_ports = 0;
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 371 struct device *dev = &pdev->dev;
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 372 struct ahci_host_priv *hpriv;
b1a9edbda040a43 Antoine Tenart 2014-07-30 373 struct device_node *child;
b1a9edbda040a43 Antoine Tenart 2014-07-30 374 u32 mask_port_map = 0;
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 375
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 376 if (!devres_open_group(dev, NULL, GFP_KERNEL))
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 377 return ERR_PTR(-ENOMEM);
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 378
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 379 hpriv = devres_alloc(ahci_platform_put_resources, sizeof(*hpriv),
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 380 GFP_KERNEL);
09d29e57ad01111 Serge Semin 2022-05-12 381 if (!hpriv) {
09d29e57ad01111 Serge Semin 2022-05-12 @382 rc = -ENOMEM;
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 383 goto err_out;
09d29e57ad01111 Serge Semin 2022-05-12 384 }
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 385
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 386 devres_add(dev, hpriv);
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 387
a28445fba62bae3 Serge Semin 2022-05-12 388 /*
a28445fba62bae3 Serge Semin 2022-05-12 389 * If the DT provided an "ahci" named resource, use it. Otherwise,
a28445fba62bae3 Serge Semin 2022-05-12 390 * fallback to using the default first resource for the device node.
a28445fba62bae3 Serge Semin 2022-05-12 391 */
a28445fba62bae3 Serge Semin 2022-05-12 392 if (platform_get_resource_byname(pdev, IORESOURCE_MEM, "ahci"))
a28445fba62bae3 Serge Semin 2022-05-12 393 hpriv->mmio = devm_platform_ioremap_resource_byname(pdev, "ahci");
a28445fba62bae3 Serge Semin 2022-05-12 394 else
a28445fba62bae3 Serge Semin 2022-05-12 395 hpriv->mmio = devm_platform_ioremap_resource(pdev, 0);
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 396 if (IS_ERR(hpriv->mmio)) {
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 397 rc = PTR_ERR(hpriv->mmio);
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 398 goto err_out;
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 399 }
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 400
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 401 /*
71066bfbd7d3a8c Serge Semin 2022-05-12 402 * Bulk clocks getting procedure can fail to find any clock due to
71066bfbd7d3a8c Serge Semin 2022-05-12 403 * running on a non-OF platform or due to the clocks being defined in
71066bfbd7d3a8c Serge Semin 2022-05-12 404 * bypass of the DT firmware (like da850, spear13xx). In that case we
71066bfbd7d3a8c Serge Semin 2022-05-12 405 * fallback to getting a single clock source right from the dev clocks
71066bfbd7d3a8c Serge Semin 2022-05-12 406 * list.
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 407 */
71066bfbd7d3a8c Serge Semin 2022-05-12 408 rc = devm_clk_bulk_get_all(dev, &hpriv->clks);
71066bfbd7d3a8c Serge Semin 2022-05-12 409 if (rc < 0)
71066bfbd7d3a8c Serge Semin 2022-05-12 410 goto err_out;
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 411
71066bfbd7d3a8c Serge Semin 2022-05-12 412 if (rc > 0) {
71066bfbd7d3a8c Serge Semin 2022-05-12 413 /* Got clocks in bulk */
71066bfbd7d3a8c Serge Semin 2022-05-12 414 hpriv->n_clks = rc;
71066bfbd7d3a8c Serge Semin 2022-05-12 415 } else {
71066bfbd7d3a8c Serge Semin 2022-05-12 416 /*
71066bfbd7d3a8c Serge Semin 2022-05-12 417 * No clock bulk found: fallback to manually getting
71066bfbd7d3a8c Serge Semin 2022-05-12 418 * the optional clock.
71066bfbd7d3a8c Serge Semin 2022-05-12 419 */
71066bfbd7d3a8c Serge Semin 2022-05-12 420 hpriv->clks = devm_kzalloc(dev, sizeof(*hpriv->clks), GFP_KERNEL);
71066bfbd7d3a8c Serge Semin 2022-05-12 421 if (!hpriv->clks) {
71066bfbd7d3a8c Serge Semin 2022-05-12 422 rc = -ENOMEM;
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 423 goto err_out;
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 424 }
71066bfbd7d3a8c Serge Semin 2022-05-12 425 hpriv->clks->clk = devm_clk_get_optional(dev, NULL);
71066bfbd7d3a8c Serge Semin 2022-05-12 426 if (IS_ERR(hpriv->clks->clk)) {
71066bfbd7d3a8c Serge Semin 2022-05-12 427 rc = PTR_ERR(hpriv->clks->clk);
71066bfbd7d3a8c Serge Semin 2022-05-12 428 goto err_out;
71066bfbd7d3a8c Serge Semin 2022-05-12 429 } else if (hpriv->clks->clk) {
71066bfbd7d3a8c Serge Semin 2022-05-12 430 hpriv->clks->id = __clk_get_name(hpriv->clks->clk);
71066bfbd7d3a8c Serge Semin 2022-05-12 431 hpriv->n_clks = 1;
71066bfbd7d3a8c Serge Semin 2022-05-12 432 }
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 433 }
fd990556f0fa254 Bartlomiej Zolnierkiewicz 2014-03-25 434
962399bb7fbf5ce Mark Brown 2019-10-16 435 hpriv->ahci_regulator = devm_regulator_get(dev, "ahci");
a37da9180f42c95 Corentin Labbe 2018-09-03 436 if (IS_ERR(hpriv->ahci_regulator)) {
a37da9180f42c95 Corentin Labbe 2018-09-03 437 rc = PTR_ERR(hpriv->ahci_regulator);
962399bb7fbf5ce Mark Brown 2019-10-16 438 if (rc != 0)
a37da9180f42c95 Corentin Labbe 2018-09-03 439 goto err_out;
a37da9180f42c95 Corentin Labbe 2018-09-03 440 }
a37da9180f42c95 Corentin Labbe 2018-09-03 441
962399bb7fbf5ce Mark Brown 2019-10-16 442 hpriv->phy_regulator = devm_regulator_get(dev, "phy");
f20fb266e77a8af Corentin Labbe 2018-09-03 443 if (IS_ERR(hpriv->phy_regulator)) {
f20fb266e77a8af Corentin Labbe 2018-09-03 444 rc = PTR_ERR(hpriv->phy_regulator);
f20fb266e77a8af Corentin Labbe 2018-09-03 445 goto err_out;
f20fb266e77a8af Corentin Labbe 2018-09-03 446 }
f20fb266e77a8af Corentin Labbe 2018-09-03 447
9d2ab9957397083 Kunihiko Hayashi 2018-08-22 448 if (flags & AHCI_PLATFORM_GET_RESETS) {
9d2ab9957397083 Kunihiko Hayashi 2018-08-22 449 hpriv->rsts = devm_reset_control_array_get_optional_shared(dev);
9d2ab9957397083 Kunihiko Hayashi 2018-08-22 450 if (IS_ERR(hpriv->rsts)) {
9d2ab9957397083 Kunihiko Hayashi 2018-08-22 451 rc = PTR_ERR(hpriv->rsts);
9d2ab9957397083 Kunihiko Hayashi 2018-08-22 452 goto err_out;
9d2ab9957397083 Kunihiko Hayashi 2018-08-22 453 }
9d2ab9957397083 Kunihiko Hayashi 2018-08-22 454 }
9d2ab9957397083 Kunihiko Hayashi 2018-08-22 455
c7d7ddee7e24eed Gregory CLEMENT 2015-01-15 456 hpriv->nports = child_nodes = of_get_child_count(dev->of_node);
b1a9edbda040a43 Antoine Tenart 2014-07-30 457
c7d7ddee7e24eed Gregory CLEMENT 2015-01-15 458 /*
c7d7ddee7e24eed Gregory CLEMENT 2015-01-15 459 * If no sub-node was found, we still need to set nports to
c7d7ddee7e24eed Gregory CLEMENT 2015-01-15 460 * one in order to be able to use the
c7d7ddee7e24eed Gregory CLEMENT 2015-01-15 461 * ahci_platform_[en|dis]able_[phys|regulators] functions.
c7d7ddee7e24eed Gregory CLEMENT 2015-01-15 462 */
c7d7ddee7e24eed Gregory CLEMENT 2015-01-15 463 if (!child_nodes)
c7d7ddee7e24eed Gregory CLEMENT 2015-01-15 464 hpriv->nports = 1;
c7d7ddee7e24eed Gregory CLEMENT 2015-01-15 465
a4b9f5ed02e2351 Corentin Labbe 2018-07-12 466 hpriv->phys = devm_kcalloc(dev, hpriv->nports, sizeof(*hpriv->phys), GFP_KERNEL);
b1a9edbda040a43 Antoine Tenart 2014-07-30 467 if (!hpriv->phys) {
b1a9edbda040a43 Antoine Tenart 2014-07-30 468 rc = -ENOMEM;
b1a9edbda040a43 Antoine Tenart 2014-07-30 469 goto err_out;
b1a9edbda040a43 Antoine Tenart 2014-07-30 470 }
04ba9488199e3ee Corentin Labbe 2018-07-17 471 /*
04ba9488199e3ee Corentin Labbe 2018-07-17 472 * We cannot use devm_ here, since ahci_platform_put_resources() uses
04ba9488199e3ee Corentin Labbe 2018-07-17 473 * target_pwrs after devm_ have freed memory
04ba9488199e3ee Corentin Labbe 2018-07-17 474 */
04ba9488199e3ee Corentin Labbe 2018-07-17 475 hpriv->target_pwrs = kcalloc(hpriv->nports, sizeof(*hpriv->target_pwrs), GFP_KERNEL);
c7d7ddee7e24eed Gregory CLEMENT 2015-01-15 476 if (!hpriv->target_pwrs) {
c7d7ddee7e24eed Gregory CLEMENT 2015-01-15 477 rc = -ENOMEM;
c7d7ddee7e24eed Gregory CLEMENT 2015-01-15 478 goto err_out;
c7d7ddee7e24eed Gregory CLEMENT 2015-01-15 479 }
b1a9edbda040a43 Antoine Tenart 2014-07-30 480
c7d7ddee7e24eed Gregory CLEMENT 2015-01-15 481 if (child_nodes) {
b1a9edbda040a43 Antoine Tenart 2014-07-30 482 for_each_child_of_node(dev->of_node, child) {
b1a9edbda040a43 Antoine Tenart 2014-07-30 483 u32 port;
f627cfdeb7d07df Guenter Roeck 2015-01-31 484 struct platform_device *port_dev __maybe_unused;
b1a9edbda040a43 Antoine Tenart 2014-07-30 485
b1a9edbda040a43 Antoine Tenart 2014-07-30 486 if (!of_device_is_available(child))
b1a9edbda040a43 Antoine Tenart 2014-07-30 487 continue;
b1a9edbda040a43 Antoine Tenart 2014-07-30 488
b1a9edbda040a43 Antoine Tenart 2014-07-30 489 if (of_property_read_u32(child, "reg", &port)) {
b1a9edbda040a43 Antoine Tenart 2014-07-30 490 rc = -EINVAL;
d7f76f36a8b4dc8 Nishka Dasgupta 2019-08-15 491 of_node_put(child);
b1a9edbda040a43 Antoine Tenart 2014-07-30 492 goto err_out;
b1a9edbda040a43 Antoine Tenart 2014-07-30 493 }
b1a9edbda040a43 Antoine Tenart 2014-07-30 494
b1a9edbda040a43 Antoine Tenart 2014-07-30 495 if (port >= hpriv->nports) {
b1a9edbda040a43 Antoine Tenart 2014-07-30 496 dev_warn(dev, "invalid port number %d\n", port);
b1a9edbda040a43 Antoine Tenart 2014-07-30 497 continue;
b1a9edbda040a43 Antoine Tenart 2014-07-30 498 }
b1a9edbda040a43 Antoine Tenart 2014-07-30 499 mask_port_map |= BIT(port);
b1a9edbda040a43 Antoine Tenart 2014-07-30 500
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On 5/12/22 01:17, Serge Semin wrote:
> In order to simplify the clock-related code there is a way to convert the
> current fixed clocks array into using the common bulk clocks kernel API
> with dynamic set of the clock handlers and device-managed clock-resource
> tracking. It's a bit tricky due to the complication coming from the
> requirement to support the platforms (da850, spear13xx) with the
> non-OF-based clock source, but still doable.
>
> Before this modification there are two methods have been used to get the
> clocks connected to an AHCI device: clk_get() - to get the very first
> clock in the list and of_clk_get() - to get the rest of them. Basically
> the platforms with non-OF-based clocks definition could specify only a
> single reference clock source. The platforms with OF-hw clocks have been
> luckier and could setup up to AHCI_MAX_CLKS clocks. Such semantic can be
> retained with using devm_clk_bulk_get_all() to retrieve the clocks defined
> via the DT firmware and devm_clk_get_optional() otherwise. In both cases
> using the device-managed version of the methods will cause the automatic
> resources deallocation on the AHCI device removal event. The only
> complicated part in the suggested approach is the explicit allocation and
> initialization of the clk_bulk_data structure instance for the non-OF
> reference clocks. It's required in order to use the Bulk Clocks API for
> the both denoted cases of the clocks definition.
>
> Note aside with the clock-related code reduction and natural
> simplification, there are several bonuses the suggested modification
> provides. First of all the limitation of having no greater than
> AHCI_MAX_CLKS clocks is now removed, since the devm_clk_bulk_get_all()
> method will allocate as many reference clocks data descriptors as there
> are clocks specified for the device. Secondly the clock names are
> auto-detected. So the LLDD (glue) drivers can make sure that the required
> clocks are specified just by checking the clock IDs in the clk_bulk_data
> array. Thirdly using the handy Bulk Clocks kernel API improves the
> clocks-handling code readability. And the last but not least this
> modification implements a true optional clocks support to the
> ahci_platform_get_resources() method. Indeed the previous clocks getting
> procedure just stopped getting the clocks on any errors (aside from
> non-critical -EPROBE_DEFER) in a way so the callee wasn't even informed
> about abnormal loop termination. The new implementation lacks of such
> problem. The ahci_platform_get_resources() will return an error code if
> the corresponding clocks getting method ends execution abnormally.
>
> Signed-off-by: Serge Semin <[email protected]>
>
> ---
>
> Changelog v2:
> - Convert to checking the error-case first in the devm_clk_bulk_get_all()
> method invocation. (@Damien)
> - Fix some grammar mistakes in the comments.
> ---
> drivers/ata/ahci.h | 4 +-
> drivers/ata/libahci_platform.c | 84 ++++++++++++++++------------------
> 2 files changed, 41 insertions(+), 47 deletions(-)
>
Reviewed-by: Hannes Reinecke <[email protected]>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
[email protected] +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer