On Tue, Jun 10, 2025 at 04:09:32PM +0530, Anshuman Khandual wrote:
On 09/06/25 10:44 PM, Leo Yan wrote:
On Mon, Jun 09, 2025 at 05:58:34PM +0100, Suzuki Kuruppassery Poulose wrote:
[...]
static inline struct clk *coresight_get_enable_apb_pclk(struct device *dev) {
- struct clk *pclk;
- struct clk *pclk = NULL;
- pclk = devm_clk_get_enabled(dev, "apb_pclk");
- if (IS_ERR(pclk))
pclk = devm_clk_get_enabled(dev, "apb");
- if (!dev_is_amba(dev)) {
pclk = devm_clk_get_enabled(dev, "apb_pclk");if (IS_ERR(pclk))pclk = devm_clk_get_enabled(dev, "apb");AMBA driver doesn't handle "apb" clock ? So we may need to retain that here ?
Here checks the condition "if (!dev_is_amba(dev))", it means the device is not an AMBA device (e.g., a platform device), the APB clock is enabled at here.
Just exit early for AMBA devices when 'pclk' clock is still NULL ?
if (dev_is_amba(dev)) return pclk;
If it is an AMBA device, we should return a NULL pointer, as this indicates that the APB clock is not managed by the CoreSight driver.
In this patch, I did not perform any refactoring and simply made a straightforward changed. The refactoring is done in the patch 07, as you suggested, where the function is refined as:
if (dev_is_amba(dev)) { return NULL; } else { pclk = devm_clk_get_enabled(dev, "apb_pclk"); ... }
Would it be acceptable to keep this patch as it is?
Thanks, Leo