Received-SPF: pass (google.com: domain of linux-kernel+bounces-93871-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: "Shradha Todi" <shradha.t@samsung.com>
To: "'Dan Carpenter'" <dan.carpenter@linaro.org>
Cc: <linux-clk@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-pci@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>,
	<linux-samsung-soc@vger.kernel.org>, <mturquette@baylibre.com>,
	<sboyd@kernel.org>, <jingoohan1@gmail.com>, <lpieralisi@kernel.org>,
	<kw@linux.com>, <robh@kernel.org>, <bhelgaas@google.com>,
	<krzysztof.kozlowski@linaro.org>, <alim.akhtar@samsung.com>,
	<linux@armlinux.org.uk>, <m.szyprowski@samsung.com>,
	<manivannan.sadhasivam@linaro.org>, <pankaj.dubey@samsung.com>,
	<gost.dev@samsung.com>
In-Reply-To: <f00eed31-4baf-4d5c-934d-8223d1ab554d@moroto.mountain>
Subject: RE: [PATCH v6 1/2] clk: Provide managed helper to get and enable
 bulk clocks
Date: Wed, 6 Mar 2024 17:43:03 +0530
Message-ID: <022301da6fbf$aae4f7e0$00aee7a0$@samsung.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Thread-Index: AQH2hJCcwYDjJDqbclltz640cKP7jQL/AcmgAaOboDkDexFUCLCxgc9A
Content-Language: en-in
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 105P
DLP-Filter: Pass
References: <20240220084046.23786-1-shradha.t@samsung.com>
	<CGME20240220084120epcas5p1e8980539667c3d9da20f49fc645d8f4c@epcas5p1.samsung.com>
	<20240220084046.23786-2-shradha.t@samsung.com>
	<f00eed31-4baf-4d5c-934d-8223d1ab554d@moroto.mountain>


> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@linaro.org>
> Sent: 05 March 2024 14:20
> To: Shradha Todi <shradha.t@samsung.com>
> Cc: linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-samsung-
> soc@vger.kernel.org; mturquette@baylibre.com; sboyd@kernel.org;
> jingoohan1@gmail.com; lpieralisi@kernel.org; kw@linux.com; robh@kernel.org;
> bhelgaas@google.com; krzysztof.kozlowski@linaro.org;
> alim.akhtar@samsung.com; linux@armlinux.org.uk;
> m.szyprowski@samsung.com; manivannan.sadhasivam@linaro.org;
> pankaj.dubey@samsung.com; gost.dev@samsung.com
> Subject: Re: [PATCH v6 1/2] clk: Provide managed helper to get and enable bulk
> clocks
> 
> On Tue, Feb 20, 2024 at 02:10:45PM +0530, Shradha Todi wrote:
> > +int __must_check devm_clk_bulk_get_all_enable(struct device *dev,
> > +					      struct clk_bulk_data **clks) {
> > +	struct clk_bulk_devres *devres;
> > +	int ret;
> > +
> > +	devres = devres_alloc(devm_clk_bulk_release_all_enable,
> > +			      sizeof(*devres), GFP_KERNEL);
> > +	if (!devres)
> > +		return -ENOMEM;
> > +
> > +	ret = clk_bulk_get_all(dev, &devres->clks);
> > +	if (ret > 0) {
> 
> I feel like this should be >= instead of >.  There aren't any callers of this
function
> yet so we can't see what's in *clks at the start but it's easy to imagine a
situation
> where it's bad data.
> 

Reference for this piece of code has been taken from devm_clk_bulk_get_all()
which
has multiple callers, so it's safe. If we make this >=, it will hold on to the
devres node
even though there are no clocks.

> > +		*clks = devres->clks;
> > +		devres->num_clks = ret;
> > +	} else {
> > +		devres_free(devres);
> > +		return ret;
> 
> When clk_bulk_get_all() returns zero then we return success here.
> 

Yes, we are returning success in case there are no clocks as well. In case there
are no
clocks defined in the DT-node, then it is assumed that the driver does not need
any
clock manipulation for driver operation. So the intention here is to continue
without
throwing error.

> regards,
> dan carpenter
> 
> > +	}
> > +
> > +	ret = clk_bulk_prepare_enable(devres->num_clks, *clks);
> > +	if (!ret) {
> > +		devres_add(dev, devres);
> > +	} else {
> > +		clk_bulk_put_all(devres->num_clks, devres->clks);
> > +		devres_free(devres);
> > +	}
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(devm_clk_bulk_get_all_enable);
> > +