On 13.12.2019 06:22, Chanwoo Choi wrote:
> Hi,
>
> On 11/15/19 5:09 AM, Leonard Crestez wrote:
>> There is no single device which can represent the imx interconnect.
>> Instead of adding a virtual one just make the main &noc act as the
>> global interconnect provider.
>>
>> The imx interconnect provider driver will scale the NOC and DDRC based
>> on bandwidth request. More scalable nodes can be added in the future,
>> for example for audio/display/vpu/gpu NICs.
>>
>> Signed-off-by: Leonard Crestez <leonard.crestez@xxxxxxx>
>> ---
>> drivers/devfreq/imx-devfreq.c | 37 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 37 insertions(+)
>>
>> diff --git a/drivers/devfreq/imx-devfreq.c b/drivers/devfreq/imx-devfreq.c
>> index 620b344e87aa..585d340c0f6e 100644
>> --- a/drivers/devfreq/imx-devfreq.c
>> +++ b/drivers/devfreq/imx-devfreq.c
>> @@ -15,10 +15,11 @@
>> struct imx_devfreq {
>> struct devfreq_dev_profile profile;
>> struct devfreq *devfreq;
>> struct clk *clk;
>> struct devfreq_passive_data passive_data;
>> + struct platform_device *icc_pdev;
>> };
>>
>> static int imx_devfreq_target(struct device *dev,
>> unsigned long *freq, u32 flags)
>> {
>> @@ -60,11 +61,40 @@ static int imx_devfreq_get_dev_status(struct device *dev,
>> return 0;
>> }
>>
>> static void imx_devfreq_exit(struct device *dev)
>> {
>> + struct imx_devfreq *priv = dev_get_drvdata(dev);
>> +
>> dev_pm_opp_of_remove_table(dev);
>> + platform_device_unregister(priv->icc_pdev);
>> +}
>> +
>> +/* imx_devfreq_init_icc() - register matching icc provider if required */
>> +static int imx_devfreq_init_icc(struct device *dev)
>> +{
>> + struct imx_devfreq *priv = dev_get_drvdata(dev);
>> + const char *icc_driver_name;
>> +
>> + if (!IS_ENABLED(CONFIG_INTERCONNECT_IMX))
>> + return 0;
>
> It is not proper to check the enable state of CONFIG_INTERCONNECT_IMX configuration
> on device driver. Why don't you add the 'select CONFIG_INTERCONNECT_IMX' on Kconfig?
Because it's optional.
You can disable interconnect support and just tweak frequencies using
the devfreq sysfs API. But indeed would only really be useful for debugging.
>> + if (!of_get_property(dev->of_node, "#interconnect-cells", 0))
>> + return 0;
>> +
>> + icc_driver_name = of_device_get_match_data(dev);
>> + if (!icc_driver_name)
>> + return 0;
>> +
>> + priv->icc_pdev = platform_device_register_data(
>> + dev, icc_driver_name, 0, NULL, 0);
>> + if (IS_ERR(priv->icc_pdev)) {
>> + dev_err(dev, "failed to register icc provider %s: %ld\n",
>> + icc_driver_name, PTR_ERR(priv->devfreq));
>> + return PTR_ERR(priv->devfreq);
>> + }
>> +
>> + return 0;
>> }
>>
>> static int imx_devfreq_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> @@ -120,18 +150,25 @@ static int imx_devfreq_probe(struct platform_device *pdev)
>> ret = PTR_ERR(priv->devfreq);
>> dev_err(dev, "failed to add devfreq device: %d\n", ret);
>> goto err;
>> }
>>
>> + ret = imx_devfreq_init_icc(dev);
>> + if (ret)
>> + goto err;
>> +
>> return 0;
>>
>> err:
>> dev_pm_opp_of_remove_table(dev);
>> return ret;
>> }
>>
>> static const struct of_device_id imx_devfreq_of_match[] = {
>> + { .compatible = "fsl,imx8mq-noc", .data = "imx8mq-interconnect", },
>> + { .compatible = "fsl,imx8mm-noc", .data = "imx8mm-interconnect", },
>> + { .compatible = "fsl,imx8mn-noc", .data = "imx8mn-interconnect", },
>> { .compatible = "fsl,imx8m-noc", },
>> { .compatible = "fsl,imx8m-nic", },
>> { /* sentinel */ },
>> };
>> MODULE_DEVICE_TABLE(of, imx_devfreq_of_match);
