Re: [PATCH v2 3/4] Coresight: Add Coresight Control Unit driver

[JieGan <quic_jiegan@quicinc.com>]

On Mon, Jul 08, 2024 at 12:44:43PM +0200, Krzysztof Kozlowski wrote:
> On 08/07/2024 05:16, JieGan wrote:
> > 
> >>
> >>> +
> >>> +	drvdata->base = devm_ioremap(dev, res->start, resource_size(res));
> >>
> >> Use proper wrapper for this two.
> > Replaced by:
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > drvdata->base = devm_ioremap_resource(dev, res);
> 
> Why?
> 
> Use the wrapper.
Sorry, I misunderstood the "wrapper" before.
Just wrapped drvdata->base by void __iomem *base;

> 
> 
> ...
> 
> >>> +
> >>> +static struct platform_driver ccu_driver = {
> >>> +	.probe          = ccu_probe,
> >>> +	.remove         = ccu_remove,
> >>> +	.driver         = {
> >>> +		.name   = "coresight-ccu",
> >>> +		.of_match_table = ccu_match,
> >>> +		.suppress_bind_attrs = true,
> >>
> >> Why?
> > Sorry, I dont get the point here.
> 
> You do not get the point why I am asking "why?"?
> 
> Why do you need it?
> 
> > We dont need automatic bind/unbind, so the suppress_bind_attrs sets to true.
> 
> But I need it...
> 
> > We need configure some settings before we register the device.
> 
> Hm, is this expected for coresight devices?
The coresight device cannot be unbinded independently. As we known, 
The coresight devices collaborate to form a path, from source, link, to sink.
If an unexpected unbinding occurs for a coresight device that is part of the path,
it will disrupt the entire path.

I think it's the reason why the coresight device driver does not need automatic bind/unbind.
Here is the previous discussion for suppress_bind_attrs:
https://lore.kernel.org/all/1453753248-1716-1-git-send-email-mathieu.poirier@linaro.org/#r

> 
> Best regards,
> Krzysztof
> 

Thanks,
Jie