diff mbox series

[05/21] ata: libahci_platform: Convert to using devm bulk clocks API

Message ID 20220324001628.13028-6-Sergey.Semin@baikalelectronics.ru
State New
Headers show
Series ata: ahci: Add DWC/Baikal-T1 AHCI SATA support | expand

Commit Message

Serge Semin March 24, 2022, 12:16 a.m. UTC 
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 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 <Sergey.Semin@baikalelectronics.ru>
---
 drivers/ata/ahci.h             |  4 +-
 drivers/ata/libahci_platform.c | 82 +++++++++++++++-------------------
 2 files changed, 37 insertions(+), 49 deletions(-)

Comments

kernel test robot March 28, 2022, 10:36 p.m. UTC | #1 
Hi Serge,

I love your patch! Yet something to improve:

[auto build test ERROR on v5.17]
[also build test ERROR on next-20220328]
[cannot apply to axboe-block/for-next robh/for-next linus/master]
[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/20220328-234809
base:    f443e374ae131c168a065ea1748feac6b2e76613
config: arm-buildonly-randconfig-r005-20220327 (https://download.01.org/0day-ci/archive/20220329/202203290643.0ExdJphD-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 0f6d9501cf49ce02937099350d08f20c4af86f3d)
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 arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/intel-lab-lkp/linux/commit/28cf1dcfb31bfca35af403a8774d0d880923fab3
        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/20220328-234809
        git checkout 28cf1dcfb31bfca35af403a8774d0d880923fab3
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/ata/

If you fix the issue, kindly add following tag as appropriate
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

df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   60  
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   61  static int ahci_dm816_phy_init(struct ahci_host_priv *hpriv, struct device *dev)
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   62  {
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   63  	unsigned long refclk_rate;
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   64  	int mpy;
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   65  	u32 val;
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   66  
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   67  	/*
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   68  	 * We should have been supplied two clocks: the functional and
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   69  	 * keep-alive clock and the external reference clock. We need the
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   70  	 * rate of the latter to calculate the correct value of MPY bits.
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   71  	 */
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14  @72  	if (!hpriv->clks[1]) {
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   73  		dev_err(dev, "reference clock not supplied\n");
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   74  		return -EINVAL;
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   75  	}
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   76  
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14  @77  	refclk_rate = clk_get_rate(hpriv->clks[1]);
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   78  	if ((refclk_rate % 100) != 0) {
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   79  		dev_err(dev, "reference clock rate must be divisible by 100\n");
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   80  		return -EINVAL;
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   81  	}
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   82  
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   83  	mpy = ahci_dm816_get_mpy_bits(refclk_rate);
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   84  	if (mpy < 0) {
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   85  		dev_err(dev, "can't calculate the MPY bits value\n");
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   86  		return -EINVAL;
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   87  	}
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   88  
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   89  	/* Enable the PHY and configure the first HBA port. */
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   90  	val = AHCI_DM816_PHY_MPY(mpy) | AHCI_DM816_PHY_LOS(1) |
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   91  	      AHCI_DM816_PHY_RXCDR(4) | AHCI_DM816_PHY_RXEQ(1) |
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   92  	      AHCI_DM816_PHY_TXSWING(3) | AHCI_DM816_PHY_ENPLL(1);
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   93  	writel(val, hpriv->mmio + AHCI_DM816_P0PHYCR_REG);
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   94  
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   95  	/* Configure the second HBA port. */
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   96  	val = AHCI_DM816_PHY_LOS(1) | AHCI_DM816_PHY_RXCDR(4) |
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   97  	      AHCI_DM816_PHY_RXEQ(1) | AHCI_DM816_PHY_TXSWING(3);
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   98  	writel(val, hpriv->mmio + AHCI_DM816_P1PHYCR_REG);
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14   99  
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14  100  	return 0;
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14  101  }
df46e6a4c06c89 Bartosz Golaszewski 2017-03-14  102
kernel test robot March 28, 2022, 11:42 p.m. UTC | #2 
Hi Serge,

I love your patch! Yet something to improve:

[auto build test ERROR on v5.17]
[also build test ERROR on next-20220328]
[cannot apply to axboe-block/for-next robh/for-next linus/master]
[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/20220328-234809
base:    f443e374ae131c168a065ea1748feac6b2e76613
config: alpha-randconfig-r034-20220327 (https://download.01.org/0day-ci/archive/20220329/202203290730.f6OTgSOz-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.2.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/28cf1dcfb31bfca35af403a8774d0d880923fab3
        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/20220328-234809
        git checkout 28cf1dcfb31bfca35af403a8774d0d880923fab3
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash drivers/ata/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/ata/ahci_da850.c: In function 'ahci_da850_probe':
>> drivers/ata/ahci_da850.c:181:13: error: wrong type argument to unary exclamation mark
     181 |         if (!hpriv->clks[0]) {
         |             ^
>> drivers/ata/ahci_da850.c:186:34: error: incompatible types when assigning to type 'struct clk_bulk_data' from type 'struct clk *'
     186 |                 hpriv->clks[0] = clk;
         |                                  ^~~
   drivers/ata/ahci_da850.c:194:13: error: wrong type argument to unary exclamation mark
     194 |         if (!hpriv->clks[1]) {
         |             ^
   drivers/ata/ahci_da850.c:201:34: error: incompatible types when assigning to type 'struct clk_bulk_data' from type 'struct clk *'
     201 |                 hpriv->clks[1] = clk;
         |                                  ^~~
>> drivers/ata/ahci_da850.c:204:64: error: incompatible type for argument 1 of 'clk_get_rate'
     204 |         mpy = ahci_da850_calculate_mpy(clk_get_rate(hpriv->clks[1]));
         |                                                     ~~~~~~~~~~~^~~
         |                                                                |
         |                                                                struct clk_bulk_data
   In file included from drivers/ata/ahci.h:23,
                    from drivers/ata/ahci_da850.c:13:
   include/linux/clk.h:880:54: note: expected 'struct clk *' but argument is of type 'struct clk_bulk_data'
     880 | static inline unsigned long clk_get_rate(struct clk *clk)
         |                                          ~~~~~~~~~~~~^~~
--
   drivers/ata/ahci_dm816.c: In function 'ahci_dm816_phy_init':
>> drivers/ata/ahci_dm816.c:72:13: error: wrong type argument to unary exclamation mark
      72 |         if (!hpriv->clks[1]) {
         |             ^
>> drivers/ata/ahci_dm816.c:77:47: error: incompatible type for argument 1 of 'clk_get_rate'
      77 |         refclk_rate = clk_get_rate(hpriv->clks[1]);
         |                                    ~~~~~~~~~~~^~~
         |                                               |
         |                                               struct clk_bulk_data
   In file included from drivers/ata/ahci.h:23,
                    from drivers/ata/ahci_dm816.c:16:
   include/linux/clk.h:880:54: note: expected 'struct clk *' but argument is of type 'struct clk_bulk_data'
     880 | static inline unsigned long clk_get_rate(struct clk *clk)
         |                                          ~~~~~~~~~~~~^~~


vim +186 drivers/ata/ahci_da850.c

018d5ef2048fca Akinobu Mita              2015-01-29  159  
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  160  static int ahci_da850_probe(struct platform_device *pdev)
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  161  {
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  162  	struct device *dev = &pdev->dev;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  163  	struct ahci_host_priv *hpriv;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  164  	void __iomem *pwrdn_reg;
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  165  	struct resource *res;
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  166  	struct clk *clk;
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  167  	u32 mpy;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  168  	int rc;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  169  
16af2d65842d34 Kunihiko Hayashi          2018-08-22  170  	hpriv = ahci_platform_get_resources(pdev, 0);
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  171  	if (IS_ERR(hpriv))
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  172  		return PTR_ERR(hpriv);
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  173  
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  174  	/*
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  175  	 * Internally ahci_platform_get_resources() calls clk_get(dev, NULL)
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  176  	 * when trying to obtain the functional clock. This SATA controller
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  177  	 * uses two clocks for which we specify two connection ids. If we don't
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  178  	 * have the functional clock at this point - call clk_get() again with
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  179  	 * con_id = "fck".
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  180  	 */
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30 @181  	if (!hpriv->clks[0]) {
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  182  		clk = clk_get(dev, "fck");
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  183  		if (IS_ERR(clk))
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  184  			return PTR_ERR(clk);
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  185  
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30 @186  		hpriv->clks[0] = clk;
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  187  	}
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  188  
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  189  	/*
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  190  	 * The second clock used by ahci-da850 is the external REFCLK. If we
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  191  	 * didn't get it from ahci_platform_get_resources(), let's try to
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  192  	 * specify the con_id in clk_get().
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  193  	 */
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  194  	if (!hpriv->clks[1]) {
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  195  		clk = clk_get(dev, "refclk");
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  196  		if (IS_ERR(clk)) {
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  197  			dev_err(dev, "unable to obtain the reference clock");
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  198  			return -ENODEV;
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  199  		}
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  200  
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  201  		hpriv->clks[1] = clk;
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  202  	}
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  203  
cdf0ead3747200 Bartosz Golaszewski       2017-01-30 @204  	mpy = ahci_da850_calculate_mpy(clk_get_rate(hpriv->clks[1]));
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  205  	if (mpy == 0) {
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  206  		dev_err(dev, "invalid REFCLK multiplier value: 0x%x", mpy);
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  207  		return -EINVAL;
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  208  	}
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  209  
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  210  	rc = ahci_platform_enable_resources(hpriv);
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  211  	if (rc)
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  212  		return rc;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  213  
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  214  	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
c88c094985ad38 Christophe JAILLET        2017-08-16  215  	if (!res) {
c88c094985ad38 Christophe JAILLET        2017-08-16  216  		rc = -ENODEV;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  217  		goto disable_resources;
c88c094985ad38 Christophe JAILLET        2017-08-16  218  	}
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  219  
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  220  	pwrdn_reg = devm_ioremap(dev, res->start, resource_size(res));
c88c094985ad38 Christophe JAILLET        2017-08-16  221  	if (!pwrdn_reg) {
c88c094985ad38 Christophe JAILLET        2017-08-16  222  		rc = -ENOMEM;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  223  		goto disable_resources;
c88c094985ad38 Christophe JAILLET        2017-08-16  224  	}
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  225  
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  226  	da850_sata_init(dev, pwrdn_reg, hpriv->mmio, mpy);
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  227  
018d5ef2048fca Akinobu Mita              2015-01-29  228  	rc = ahci_platform_init_host(pdev, hpriv, &ahci_da850_port_info,
018d5ef2048fca Akinobu Mita              2015-01-29  229  				     &ahci_platform_sht);
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  230  	if (rc)
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  231  		goto disable_resources;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  232  
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  233  	return 0;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  234  disable_resources:
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  235  	ahci_platform_disable_resources(hpriv);
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  236  	return rc;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  237  }
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  238
kernel test robot March 29, 2022, 12:03 a.m. UTC | #3 
Hi Serge,

I love your patch! Yet something to improve:

[auto build test ERROR on v5.17]
[also build test ERROR on next-20220328]
[cannot apply to axboe-block/for-next robh/for-next linus/master]
[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/20220328-234809
base:    f443e374ae131c168a065ea1748feac6b2e76613
config: arm-randconfig-r015-20220327 (https://download.01.org/0day-ci/archive/20220329/202203290752.NKWGfglB-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 0f6d9501cf49ce02937099350d08f20c4af86f3d)
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 arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/intel-lab-lkp/linux/commit/28cf1dcfb31bfca35af403a8774d0d880923fab3
        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/20220328-234809
        git checkout 28cf1dcfb31bfca35af403a8774d0d880923fab3
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/ata/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/ata/ahci_da850.c:181:6: error: invalid argument type 'struct clk_bulk_data' to unary expression
           if (!hpriv->clks[0]) {
               ^~~~~~~~~~~~~~~
>> drivers/ata/ahci_da850.c:186:18: error: assigning to 'struct clk_bulk_data' from incompatible type 'struct clk *'
                   hpriv->clks[0] = clk;
                                  ^ ~~~
   drivers/ata/ahci_da850.c:194:6: error: invalid argument type 'struct clk_bulk_data' to unary expression
           if (!hpriv->clks[1]) {
               ^~~~~~~~~~~~~~~
   drivers/ata/ahci_da850.c:201:18: error: assigning to 'struct clk_bulk_data' from incompatible type 'struct clk *'
                   hpriv->clks[1] = clk;
                                  ^ ~~~
>> drivers/ata/ahci_da850.c:204:46: error: passing 'struct clk_bulk_data' to parameter of incompatible type 'struct clk *'
           mpy = ahci_da850_calculate_mpy(clk_get_rate(hpriv->clks[1]));
                                                       ^~~~~~~~~~~~~~
   include/linux/clk.h:880:54: note: passing argument to parameter 'clk' here
   static inline unsigned long clk_get_rate(struct clk *clk)
                                                        ^
   5 errors generated.


vim +181 drivers/ata/ahci_da850.c

018d5ef2048fca Akinobu Mita              2015-01-29  159  
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  160  static int ahci_da850_probe(struct platform_device *pdev)
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  161  {
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  162  	struct device *dev = &pdev->dev;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  163  	struct ahci_host_priv *hpriv;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  164  	void __iomem *pwrdn_reg;
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  165  	struct resource *res;
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  166  	struct clk *clk;
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  167  	u32 mpy;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  168  	int rc;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  169  
16af2d65842d34 Kunihiko Hayashi          2018-08-22  170  	hpriv = ahci_platform_get_resources(pdev, 0);
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  171  	if (IS_ERR(hpriv))
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  172  		return PTR_ERR(hpriv);
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  173  
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  174  	/*
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  175  	 * Internally ahci_platform_get_resources() calls clk_get(dev, NULL)
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  176  	 * when trying to obtain the functional clock. This SATA controller
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  177  	 * uses two clocks for which we specify two connection ids. If we don't
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  178  	 * have the functional clock at this point - call clk_get() again with
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  179  	 * con_id = "fck".
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  180  	 */
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30 @181  	if (!hpriv->clks[0]) {
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  182  		clk = clk_get(dev, "fck");
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  183  		if (IS_ERR(clk))
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  184  			return PTR_ERR(clk);
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  185  
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30 @186  		hpriv->clks[0] = clk;
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  187  	}
82dbe1a68fd65a Bartosz Golaszewski       2017-01-30  188  
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  189  	/*
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  190  	 * The second clock used by ahci-da850 is the external REFCLK. If we
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  191  	 * didn't get it from ahci_platform_get_resources(), let's try to
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  192  	 * specify the con_id in clk_get().
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  193  	 */
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  194  	if (!hpriv->clks[1]) {
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  195  		clk = clk_get(dev, "refclk");
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  196  		if (IS_ERR(clk)) {
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  197  			dev_err(dev, "unable to obtain the reference clock");
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  198  			return -ENODEV;
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  199  		}
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  200  
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  201  		hpriv->clks[1] = clk;
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  202  	}
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  203  
cdf0ead3747200 Bartosz Golaszewski       2017-01-30 @204  	mpy = ahci_da850_calculate_mpy(clk_get_rate(hpriv->clks[1]));
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  205  	if (mpy == 0) {
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  206  		dev_err(dev, "invalid REFCLK multiplier value: 0x%x", mpy);
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  207  		return -EINVAL;
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  208  	}
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  209  
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  210  	rc = ahci_platform_enable_resources(hpriv);
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  211  	if (rc)
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  212  		return rc;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  213  
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  214  	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
c88c094985ad38 Christophe JAILLET        2017-08-16  215  	if (!res) {
c88c094985ad38 Christophe JAILLET        2017-08-16  216  		rc = -ENODEV;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  217  		goto disable_resources;
c88c094985ad38 Christophe JAILLET        2017-08-16  218  	}
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  219  
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  220  	pwrdn_reg = devm_ioremap(dev, res->start, resource_size(res));
c88c094985ad38 Christophe JAILLET        2017-08-16  221  	if (!pwrdn_reg) {
c88c094985ad38 Christophe JAILLET        2017-08-16  222  		rc = -ENOMEM;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  223  		goto disable_resources;
c88c094985ad38 Christophe JAILLET        2017-08-16  224  	}
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  225  
cdf0ead3747200 Bartosz Golaszewski       2017-01-30  226  	da850_sata_init(dev, pwrdn_reg, hpriv->mmio, mpy);
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  227  
018d5ef2048fca Akinobu Mita              2015-01-29  228  	rc = ahci_platform_init_host(pdev, hpriv, &ahci_da850_port_info,
018d5ef2048fca Akinobu Mita              2015-01-29  229  				     &ahci_platform_sht);
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  230  	if (rc)
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  231  		goto disable_resources;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  232  
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  233  	return 0;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  234  disable_resources:
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  235  	ahci_platform_disable_resources(hpriv);
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  236  	return rc;
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  237  }
ae8723f8a9c8e8 Bartlomiej Zolnierkiewicz 2014-03-25  238
Serge Semin April 9, 2022, 4:59 a.m. UTC | #4 
On Thu, Mar 24, 2022 at 10:29:01AM +0900, Damien Le Moal wrote:
> On 3/24/22 09:16, 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 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 <Sergey.Semin@baikalelectronics.ru>
> > ---
> >  drivers/ata/ahci.h             |  4 +-
> >  drivers/ata/libahci_platform.c | 82 +++++++++++++++-------------------
> >  2 files changed, 37 insertions(+), 49 deletions(-)
> > 
> > diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> > index eeac5482f1d1..1564c691094a 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,
> > @@ -341,7 +340,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 8eabbb5f208c..d805ddc3a024 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 enabled_ports = 0, rc = 0, child_nodes;
> >  	struct device *dev = &pdev->dev;
> >  	struct ahci_host_priv *hpriv;
> > -	struct clk *clk;
> >  	struct device_node *child;
> > -	int i, enabled_ports = 0, rc = 0, child_nodes;
> >  	u32 mask_port_map = 0;
> >  
> >  	if (!devres_open_group(dev, NULL, GFP_KERNEL))
> > @@ -413,25 +394,32 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
> >  		}
> >  	}
> >  
> > -	for (i = 0; i < AHCI_MAX_CLKS; i++) {
> > -		/*
> > -		 * 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).
> > -		 */
> > -		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;
> > +	/*
> > +	 * Bulk clock get procedure can fail to find any clock due to running
> > +	 * an a non-OF platform or due to the clocks being defined in bypass
> > +	 * from 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);
> 

> I would move the error check first here to make things more readable:

Agreed. Good note.

> 
> 	rc = devm_clk_bulk_get_all(dev, &hpriv->clks);
> 	if (rc < 0)
> 		goto err_out;
> 
> 	if (rc) {
> 		/* Got clocks in bulk */
> 		hpriv->n_clks = rc;
> 	} else {
> 		/*
> 		 * No clock bulk found: fallback to manually getting
> 		 * the optional clock.
> 		 */
> 		hpriv->clks = devm_kzalloc(dev, sizeof(*hpriv->clks),
> 					   GFP_KERNEL);
> 		...
> 	}
> 
> And it may be cleaner to move this entire code hunk into a helper,
> something like ahci_platform_get_clks() ?

I'd rather keep the code embedded seeing it won't be used anywhere
than here and in order to keep the ahci_platform_get_resources()
function more-or-less coherent.  Otherwise moving just a part of the
function would be a half-measure since the methods like
ahci_platform_get_regs(), ahci_platform_get_regulators(), etc could be
also unpinned.

-Sergey

> 
> > +	if (rc > 0) {
> > +		hpriv->n_clks = rc;
> > +	} else if (!rc) {
> > +		hpriv->clks = devm_kzalloc(dev, sizeof(*hpriv->clks), GFP_KERNEL);
> > +		if (!hpriv->clks) {
> > +			rc = -ENOMEM;
> > +			goto err_out;
> >  		}
> > -		hpriv->clks[i] = clk;
> > +		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;
> > +		}
> > +	} else {
> > +		goto err_out;
> >  	}
> >  
> >  	hpriv->ahci_regulator = devm_regulator_get(dev, "ahci");
> 
> 
> -- 
> Damien Le Moal
> Western Digital Research
diff mbox series

Patch

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index eeac5482f1d1..1564c691094a 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,
@@ -341,7 +340,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 8eabbb5f208c..d805ddc3a024 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 enabled_ports = 0, rc = 0, child_nodes;
 	struct device *dev = &pdev->dev;
 	struct ahci_host_priv *hpriv;
-	struct clk *clk;
 	struct device_node *child;
-	int i, enabled_ports = 0, rc = 0, child_nodes;
 	u32 mask_port_map = 0;
 
 	if (!devres_open_group(dev, NULL, GFP_KERNEL))
@@ -413,25 +394,32 @@  struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
 		}
 	}
 
-	for (i = 0; i < AHCI_MAX_CLKS; i++) {
-		/*
-		 * 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).
-		 */
-		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;
+	/*
+	 * Bulk clock get procedure can fail to find any clock due to running
+	 * an a non-OF platform or due to the clocks being defined in bypass
+	 * from 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) {
+		hpriv->n_clks = rc;
+	} else if (!rc) {
+		hpriv->clks = devm_kzalloc(dev, sizeof(*hpriv->clks), GFP_KERNEL);
+		if (!hpriv->clks) {
+			rc = -ENOMEM;
+			goto err_out;
 		}
-		hpriv->clks[i] = clk;
+		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;
+		}
+	} else {
+		goto err_out;
 	}
 
 	hpriv->ahci_regulator = devm_regulator_get(dev, "ahci");