Message ID | 20220713052917.27036-8-Sergey.Semin@baikalelectronics.ru |
---|---|
State | New |
Headers | show |
Series | ata: ahci: Add DWC/Baikal-T1 AHCI SATA support | expand |
Hi Serge, I love your patch! Yet something to improve: [auto build test ERROR on robh/for-next] [also build test ERROR on axboe-block/for-next linus/master v5.19-rc6 next-20220714] [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#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Serge-Semin/ata-ahci-Add-DWC-Baikal-T1-AHCI-SATA-support/20220713-133437 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next config: riscv-randconfig-r004-20220714 (https://download.01.org/0day-ci/archive/20220715/202207150410.A4kg5upp-lkp@intel.com/config) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 5e61b9c556267086ef9b743a0b57df302eef831b) 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 # install riscv cross compiling tool for clang build # apt-get install binutils-riscv64-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/7225145d9ff95641c04bdc1dd85915be6cd5ce57 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/20220713-133437 git checkout 7225145d9ff95641c04bdc1dd85915be6cd5ce57 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/ata/ If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> drivers/ata/ahci_dm816.c:72:6: error: invalid argument type 'struct clk_bulk_data' to unary expression if (!hpriv->clks[1]) { ^~~~~~~~~~~~~~~ >> drivers/ata/ahci_dm816.c:77:29: error: passing 'struct clk_bulk_data' to parameter of incompatible type 'struct clk *' refclk_rate = clk_get_rate(hpriv->clks[1]); ^~~~~~~~~~~~~~ include/linux/clk.h:584:40: note: passing argument to parameter 'clk' here unsigned long clk_get_rate(struct clk *clk); ^ 2 errors generated. vim +72 drivers/ata/ahci_dm816.c df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 60 df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 61 static int ahci_dm816_phy_init(struct ahci_host_priv *hpriv, struct device *dev) df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 62 { df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 63 unsigned long refclk_rate; df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 64 int mpy; df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 65 u32 val; df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 66 df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 67 /* df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 68 * We should have been supplied two clocks: the functional and df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 69 * keep-alive clock and the external reference clock. We need the df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 70 * rate of the latter to calculate the correct value of MPY bits. df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 71 */ df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 @72 if (!hpriv->clks[1]) { df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 73 dev_err(dev, "reference clock not supplied\n"); df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 74 return -EINVAL; df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 75 } df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 76 df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 @77 refclk_rate = clk_get_rate(hpriv->clks[1]); df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 78 if ((refclk_rate % 100) != 0) { df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 79 dev_err(dev, "reference clock rate must be divisible by 100\n"); df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 80 return -EINVAL; df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 81 } df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 82 df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 83 mpy = ahci_dm816_get_mpy_bits(refclk_rate); df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 84 if (mpy < 0) { df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 85 dev_err(dev, "can't calculate the MPY bits value\n"); df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 86 return -EINVAL; df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 87 } df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 88 df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 89 /* Enable the PHY and configure the first HBA port. */ df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 90 val = AHCI_DM816_PHY_MPY(mpy) | AHCI_DM816_PHY_LOS(1) | df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 91 AHCI_DM816_PHY_RXCDR(4) | AHCI_DM816_PHY_RXEQ(1) | df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 92 AHCI_DM816_PHY_TXSWING(3) | AHCI_DM816_PHY_ENPLL(1); df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 93 writel(val, hpriv->mmio + AHCI_DM816_P0PHYCR_REG); df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 94 df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 95 /* Configure the second HBA port. */ df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 96 val = AHCI_DM816_PHY_LOS(1) | AHCI_DM816_PHY_RXCDR(4) | df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 97 AHCI_DM816_PHY_RXEQ(1) | AHCI_DM816_PHY_TXSWING(3); df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 98 writel(val, hpriv->mmio + AHCI_DM816_P1PHYCR_REG); df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 99 df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 100 return 0; df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 101 } df46e6a4c06c89a Bartosz Golaszewski 2017-03-14 102
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 1e9e825d6cc5..814804582d1d 100644 --- a/drivers/ata/libahci_platform.c +++ b/drivers/ata/libahci_platform.c @@ -8,6 +8,7 @@ * Anton Vorontsov <avorontsov@ru.mvista.com> */ +#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 child_nodes, rc = -ENOMEM, 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 = -ENOMEM, child_nodes; u32 mask_port_map = 0; if (!devres_open_group(dev, NULL, GFP_KERNEL)) @@ -415,25 +396,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");