diff mbox series

[2/2] remoteproc: Ingenic: Add support for new Ingenic SoCs.

Message ID 1627117898-125239-3-git-send-email-zhouyanjie@wanyeetech.com (mailing list archive)
State Not Applicable
Headers show
Series Add VPU support for new Ingenic SoCs. | expand

Commit Message

Zhou Yanjie July 24, 2021, 9:11 a.m. UTC
Add support for probing the ingenic_rproc driver on the JZ4760 SoC,
the JZ4760B SoC, the JZ4775 SoC, and the JZ4780 SoC from Ingenic.

Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---
 drivers/remoteproc/ingenic_rproc.c | 115 +++++++++++++++++++++++++++++--------
 1 file changed, 91 insertions(+), 24 deletions(-)

Comments

Paul Cercueil July 24, 2021, 11:15 a.m. UTC | #1
Hi Zhou,

Le sam., juil. 24 2021 at 17:11:38 +0800, 周琰杰 (Zhou Yanjie) 
<zhouyanjie@wanyeetech.com> a écrit :
> Add support for probing the ingenic_rproc driver on the JZ4760 SoC,
> the JZ4760B SoC, the JZ4775 SoC, and the JZ4780 SoC from Ingenic.
> 
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> ---
>  drivers/remoteproc/ingenic_rproc.c | 115 
> +++++++++++++++++++++++++++++--------
>  1 file changed, 91 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/remoteproc/ingenic_rproc.c 
> b/drivers/remoteproc/ingenic_rproc.c
> index a356738..6a2e864 100644
> --- a/drivers/remoteproc/ingenic_rproc.c
> +++ b/drivers/remoteproc/ingenic_rproc.c
> @@ -2,6 +2,7 @@
>  /*
>   * Ingenic JZ47xx remoteproc driver
>   * Copyright 2019, Paul Cercueil <paul@crapouillou.net>
> + * Copyright 2021, 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@wanyeetech.com>
>   */
> 
>  #include <linux/bitops.h>
> @@ -17,7 +18,7 @@
> 
>  #define REG_AUX_CTRL		0x0
>  #define REG_AUX_MSG_ACK		0x10
> -#define REG_AUX_MSG		0x14
> +#define REG_AUX_MSG			0x14
>  #define REG_CORE_MSG_ACK	0x18
>  #define REG_CORE_MSG		0x1C
> 
> @@ -32,6 +33,20 @@ module_param(auto_boot, bool, 0400);
>  MODULE_PARM_DESC(auto_boot,
>  		 "Auto-boot the remote processor [default=false]");
> 
> +enum ingenic_vpu_version {
> +	ID_JZ4760,
> +	ID_JZ4770,
> +	ID_JZ4775,
> +};

The "version" field of ingenic_so_cinfo is not used anywhere, so you 
can drop this enum completely.

> +
> +struct ingenic_soc_info {
> +	enum ingenic_vpu_version version;
> +	const struct vpu_mem_map *mem_map;
> +
> +	unsigned int num_clks;
> +	unsigned int num_mems;
> +};
> +
>  struct vpu_mem_map {
>  	const char *name;
>  	unsigned int da;
> @@ -43,26 +58,21 @@ struct vpu_mem_info {
>  	void __iomem *base;
>  };
> 
> -static const struct vpu_mem_map vpu_mem_map[] = {
> -	{ "tcsm0", 0x132b0000 },
> -	{ "tcsm1", 0xf4000000 },
> -	{ "sram",  0x132f0000 },
> -};
> -
>  /**
>   * struct vpu - Ingenic VPU remoteproc private structure
>   * @irq: interrupt number
>   * @clks: pointers to the VPU and AUX clocks
>   * @aux_base: raw pointer to the AUX interface registers
> - * @mem_info: array of struct vpu_mem_info, which contain the 
> mapping info of
> + * @mem_info: pointers to the struct vpu_mem_info, which contain the 
> mapping info of
>   *            each of the external memories
>   * @dev: private pointer to the device
>   */
>  struct vpu {
>  	int irq;
> -	struct clk_bulk_data clks[2];
>  	void __iomem *aux_base;
> -	struct vpu_mem_info mem_info[ARRAY_SIZE(vpu_mem_map)];
> +	const struct ingenic_soc_info *soc_info;
> +	struct clk_bulk_data *clks;
> +	struct vpu_mem_info *mem_info;
>  	struct device *dev;
>  };
> 
> @@ -72,7 +82,7 @@ static int ingenic_rproc_prepare(struct rproc 
> *rproc)
>  	int ret;
> 
>  	/* The clocks must be enabled for the firmware to be loaded in TCSM 
> */
> -	ret = clk_bulk_prepare_enable(ARRAY_SIZE(vpu->clks), vpu->clks);
> +	ret = clk_bulk_prepare_enable(vpu->soc_info->num_clks, vpu->clks);
>  	if (ret)
>  		dev_err(vpu->dev, "Unable to start clocks: %d\n", ret);
> 
> @@ -83,7 +93,7 @@ static int ingenic_rproc_unprepare(struct rproc 
> *rproc)
>  {
>  	struct vpu *vpu = rproc->priv;
> 
> -	clk_bulk_disable_unprepare(ARRAY_SIZE(vpu->clks), vpu->clks);
> +	clk_bulk_disable_unprepare(vpu->soc_info->num_clks, vpu->clks);
> 
>  	return 0;
>  }
> @@ -127,7 +137,7 @@ static void *ingenic_rproc_da_to_va(struct rproc 
> *rproc, u64 da, size_t len, boo
>  	void __iomem *va = NULL;
>  	unsigned int i;
> 
> -	for (i = 0; i < ARRAY_SIZE(vpu_mem_map); i++) {
> +	for (i = 0; i < vpu->soc_info->num_mems; i++) {
>  		const struct vpu_mem_info *info = &vpu->mem_info[i];
>  		const struct vpu_mem_map *map = info->map;
> 
> @@ -163,8 +173,60 @@ static irqreturn_t vpu_interrupt(int irq, void 
> *data)
>  	return rproc_vq_interrupt(rproc, vring);
>  }
> 
> +static const struct vpu_mem_map jz4760_vpu_mem_map[] = {
> +	{ "tcsm0", 0x132b0000 },
> +	{ "tcsm1", 0xf4000000 },
> +	{ "sram",  0x132d0000 },
> +};
> +
> +static const struct vpu_mem_map jz4770_vpu_mem_map[] = {
> +	{ "tcsm0", 0x132b0000 },
> +	{ "tcsm1", 0xf4000000 },
> +	{ "sram",  0x132f0000 },
> +};
> +
> +static const struct vpu_mem_map jz4775_vpu_mem_map[] = {
> +	{ "tcsm",  0xf4000000 },
> +	{ "sram",  0x132f0000 },
> +};
> +
> +static const struct ingenic_soc_info jz4760_soc_info = {
> +	.version = ID_JZ4760,
> +	.mem_map = jz4760_vpu_mem_map,
> +
> +	.num_clks = 2,
> +	.num_mems = 3,

.num_mems = ARRAY_SIZE(jz4760_vpu_mem_map),

And the same for the other ingenic_soc_info below.

> +};
> +
> +static const struct ingenic_soc_info jz4770_soc_info = {
> +	.version = ID_JZ4770,
> +	.mem_map = jz4770_vpu_mem_map,
> +
> +	.num_clks = 2,
> +	.num_mems = 3,
> +};
> +
> +static const struct ingenic_soc_info jz4775_soc_info = {
> +	.version = ID_JZ4775,
> +	.mem_map = jz4775_vpu_mem_map,
> +
> +	.num_clks = 1,
> +	.num_mems = 2,
> +};
> +
> +static const struct of_device_id ingenic_rproc_of_matches[] = {
> +	{ .compatible = "ingenic,jz4760-vpu-rproc", .data = 
> &jz4760_soc_info },
> +	{ .compatible = "ingenic,jz4760b-vpu-rproc", .data = 
> &jz4760_soc_info },
> +	{ .compatible = "ingenic,jz4770-vpu-rproc", .data = 
> &jz4770_soc_info },
> +	{ .compatible = "ingenic,jz4775-vpu-rproc", .data = 
> &jz4775_soc_info },
> +	{ .compatible = "ingenic,jz4780-vpu-rproc", .data = 
> &jz4775_soc_info },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, ingenic_rproc_of_matches);
> +
>  static int ingenic_rproc_probe(struct platform_device *pdev)
>  {
> +	const struct of_device_id *id = 
> of_match_node(ingenic_rproc_of_matches, pdev->dev.of_node);
>  	struct device *dev = &pdev->dev;
>  	struct resource *mem;
>  	struct rproc *rproc;
> @@ -181,6 +243,7 @@ static int ingenic_rproc_probe(struct 
> platform_device *pdev)
> 
>  	vpu = rproc->priv;
>  	vpu->dev = &pdev->dev;
> +	vpu->soc_info = id->data;

Use of_device_get_match_data(dev).

Then you can get rid of the "id" variable, and you won't have to move 
the "ingenic_rproc_of_matches" array.

>  	platform_set_drvdata(pdev, vpu);
> 
>  	mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "aux");
> @@ -190,9 +253,13 @@ static int ingenic_rproc_probe(struct 
> platform_device *pdev)
>  		return PTR_ERR(vpu->aux_base);
>  	}
> 
> -	for (i = 0; i < ARRAY_SIZE(vpu_mem_map); i++) {
> +	vpu->mem_info = kzalloc(sizeof(struct vpu_mem_info) * 
> vpu->soc_info->num_mems, GFP_KERNEL);

You are leaking memory here.

Also, why not just fix the current mem_info array size to 3? That 
sounds way simpler.

> +	if (!vpu->mem_info)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < vpu->soc_info->num_mems; i++) {
>  		mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> -						   vpu_mem_map[i].name);
> +						   vpu->soc_info->mem_map[i].name);
> 
>  		vpu->mem_info[i].base = devm_ioremap_resource(dev, mem);
>  		if (IS_ERR(vpu->mem_info[i].base)) {
> @@ -202,13 +269,19 @@ static int ingenic_rproc_probe(struct 
> platform_device *pdev)
>  		}
> 
>  		vpu->mem_info[i].len = resource_size(mem);
> -		vpu->mem_info[i].map = &vpu_mem_map[i];
> +		vpu->mem_info[i].map = &vpu->soc_info->mem_map[i];
>  	}
> 
> +	vpu->clks = kzalloc(sizeof(struct clk_bulk_data) * 
> vpu->soc_info->num_clks, GFP_KERNEL);

Same here, the "clks" array is already size 2, so it won't be a problem 
if you have only one clock. No need to alloc "clks" dynamically.

> +	if (!vpu->clks)
> +		return -ENOMEM;
> +
>  	vpu->clks[0].id = "vpu";
> -	vpu->clks[1].id = "aux";
> 
> -	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(vpu->clks), vpu->clks);
> +	if (vpu->soc_info->version == ID_JZ4770)
> +		vpu->clks[1].id = "aux";
> +
> +	ret = devm_clk_bulk_get(dev, vpu->soc_info->num_clks, vpu->clks);
>  	if (ret) {
>  		dev_err(dev, "Failed to get clocks\n");
>  		return ret;
> @@ -235,12 +308,6 @@ static int ingenic_rproc_probe(struct 
> platform_device *pdev)
>  	return 0;
>  }
> 
> -static const struct of_device_id ingenic_rproc_of_matches[] = {
> -	{ .compatible = "ingenic,jz4770-vpu-rproc", },
> -	{}
> -};
> -MODULE_DEVICE_TABLE(of, ingenic_rproc_of_matches);

As I wrote above - you don't need to move this.

Cheers,
-Paul

> -
>  static struct platform_driver ingenic_rproc_driver = {
>  	.probe = ingenic_rproc_probe,
>  	.driver = {
> --
> 2.7.4
>
Zhou Yanjie July 24, 2021, 1:32 p.m. UTC | #2
Hi Paul,

On 2021/7/24 下午7:15, Paul Cercueil wrote:
> Hi Zhou,
>
> Le sam., juil. 24 2021 at 17:11:38 +0800, 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@wanyeetech.com> a écrit :
>> Add support for probing the ingenic_rproc driver on the JZ4760 SoC,
>> the JZ4760B SoC, the JZ4775 SoC, and the JZ4780 SoC from Ingenic.
>>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> ---
>>  drivers/remoteproc/ingenic_rproc.c | 115 
>> +++++++++++++++++++++++++++++--------
>>  1 file changed, 91 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/remoteproc/ingenic_rproc.c 
>> b/drivers/remoteproc/ingenic_rproc.c
>> index a356738..6a2e864 100644
>> --- a/drivers/remoteproc/ingenic_rproc.c
>> +++ b/drivers/remoteproc/ingenic_rproc.c
>> @@ -2,6 +2,7 @@
>>  /*
>>   * Ingenic JZ47xx remoteproc driver
>>   * Copyright 2019, Paul Cercueil <paul@crapouillou.net>
>> + * Copyright 2021, 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>   */
>>
>>  #include <linux/bitops.h>
>> @@ -17,7 +18,7 @@
>>
>>  #define REG_AUX_CTRL        0x0
>>  #define REG_AUX_MSG_ACK        0x10
>> -#define REG_AUX_MSG        0x14
>> +#define REG_AUX_MSG            0x14
>>  #define REG_CORE_MSG_ACK    0x18
>>  #define REG_CORE_MSG        0x1C
>>
>> @@ -32,6 +33,20 @@ module_param(auto_boot, bool, 0400);
>>  MODULE_PARM_DESC(auto_boot,
>>           "Auto-boot the remote processor [default=false]");
>>
>> +enum ingenic_vpu_version {
>> +    ID_JZ4760,
>> +    ID_JZ4770,
>> +    ID_JZ4775,
>> +};
>
> The "version" field of ingenic_so_cinfo is not used anywhere, so you 
> can drop this enum completely.


Sure, I will remove it.


>
>> +
>> +struct ingenic_soc_info {
>> +    enum ingenic_vpu_version version;
>> +    const struct vpu_mem_map *mem_map;
>> +
>> +    unsigned int num_clks;
>> +    unsigned int num_mems;
>> +};
>> +
>>  struct vpu_mem_map {
>>      const char *name;
>>      unsigned int da;
>> @@ -43,26 +58,21 @@ struct vpu_mem_info {
>>      void __iomem *base;
>>  };
>>
>> -static const struct vpu_mem_map vpu_mem_map[] = {
>> -    { "tcsm0", 0x132b0000 },
>> -    { "tcsm1", 0xf4000000 },
>> -    { "sram",  0x132f0000 },
>> -};
>> -
>>  /**
>>   * struct vpu - Ingenic VPU remoteproc private structure
>>   * @irq: interrupt number
>>   * @clks: pointers to the VPU and AUX clocks
>>   * @aux_base: raw pointer to the AUX interface registers
>> - * @mem_info: array of struct vpu_mem_info, which contain the 
>> mapping info of
>> + * @mem_info: pointers to the struct vpu_mem_info, which contain the 
>> mapping info of
>>   *            each of the external memories
>>   * @dev: private pointer to the device
>>   */
>>  struct vpu {
>>      int irq;
>> -    struct clk_bulk_data clks[2];
>>      void __iomem *aux_base;
>> -    struct vpu_mem_info mem_info[ARRAY_SIZE(vpu_mem_map)];
>> +    const struct ingenic_soc_info *soc_info;
>> +    struct clk_bulk_data *clks;
>> +    struct vpu_mem_info *mem_info;
>>      struct device *dev;
>>  };
>>
>> @@ -72,7 +82,7 @@ static int ingenic_rproc_prepare(struct rproc *rproc)
>>      int ret;
>>
>>      /* The clocks must be enabled for the firmware to be loaded in 
>> TCSM */
>> -    ret = clk_bulk_prepare_enable(ARRAY_SIZE(vpu->clks), vpu->clks);
>> +    ret = clk_bulk_prepare_enable(vpu->soc_info->num_clks, vpu->clks);
>>      if (ret)
>>          dev_err(vpu->dev, "Unable to start clocks: %d\n", ret);
>>
>> @@ -83,7 +93,7 @@ static int ingenic_rproc_unprepare(struct rproc 
>> *rproc)
>>  {
>>      struct vpu *vpu = rproc->priv;
>>
>> -    clk_bulk_disable_unprepare(ARRAY_SIZE(vpu->clks), vpu->clks);
>> +    clk_bulk_disable_unprepare(vpu->soc_info->num_clks, vpu->clks);
>>
>>      return 0;
>>  }
>> @@ -127,7 +137,7 @@ static void *ingenic_rproc_da_to_va(struct rproc 
>> *rproc, u64 da, size_t len, boo
>>      void __iomem *va = NULL;
>>      unsigned int i;
>>
>> -    for (i = 0; i < ARRAY_SIZE(vpu_mem_map); i++) {
>> +    for (i = 0; i < vpu->soc_info->num_mems; i++) {
>>          const struct vpu_mem_info *info = &vpu->mem_info[i];
>>          const struct vpu_mem_map *map = info->map;
>>
>> @@ -163,8 +173,60 @@ static irqreturn_t vpu_interrupt(int irq, void 
>> *data)
>>      return rproc_vq_interrupt(rproc, vring);
>>  }
>>
>> +static const struct vpu_mem_map jz4760_vpu_mem_map[] = {
>> +    { "tcsm0", 0x132b0000 },
>> +    { "tcsm1", 0xf4000000 },
>> +    { "sram",  0x132d0000 },
>> +};
>> +
>> +static const struct vpu_mem_map jz4770_vpu_mem_map[] = {
>> +    { "tcsm0", 0x132b0000 },
>> +    { "tcsm1", 0xf4000000 },
>> +    { "sram",  0x132f0000 },
>> +};
>> +
>> +static const struct vpu_mem_map jz4775_vpu_mem_map[] = {
>> +    { "tcsm",  0xf4000000 },
>> +    { "sram",  0x132f0000 },
>> +};
>> +
>> +static const struct ingenic_soc_info jz4760_soc_info = {
>> +    .version = ID_JZ4760,
>> +    .mem_map = jz4760_vpu_mem_map,
>> +
>> +    .num_clks = 2,
>> +    .num_mems = 3,
>
> .num_mems = ARRAY_SIZE(jz4760_vpu_mem_map),
>
> And the same for the other ingenic_soc_info below.


Sure.


>
>> +};
>> +
>> +static const struct ingenic_soc_info jz4770_soc_info = {
>> +    .version = ID_JZ4770,
>> +    .mem_map = jz4770_vpu_mem_map,
>> +
>> +    .num_clks = 2,
>> +    .num_mems = 3,
>> +};
>> +
>> +static const struct ingenic_soc_info jz4775_soc_info = {
>> +    .version = ID_JZ4775,
>> +    .mem_map = jz4775_vpu_mem_map,
>> +
>> +    .num_clks = 1,
>> +    .num_mems = 2,
>> +};
>> +
>> +static const struct of_device_id ingenic_rproc_of_matches[] = {
>> +    { .compatible = "ingenic,jz4760-vpu-rproc", .data = 
>> &jz4760_soc_info },
>> +    { .compatible = "ingenic,jz4760b-vpu-rproc", .data = 
>> &jz4760_soc_info },
>> +    { .compatible = "ingenic,jz4770-vpu-rproc", .data = 
>> &jz4770_soc_info },
>> +    { .compatible = "ingenic,jz4775-vpu-rproc", .data = 
>> &jz4775_soc_info },
>> +    { .compatible = "ingenic,jz4780-vpu-rproc", .data = 
>> &jz4775_soc_info },
>> +    {}
>> +};
>> +MODULE_DEVICE_TABLE(of, ingenic_rproc_of_matches);
>> +
>>  static int ingenic_rproc_probe(struct platform_device *pdev)
>>  {
>> +    const struct of_device_id *id = 
>> of_match_node(ingenic_rproc_of_matches, pdev->dev.of_node);
>>      struct device *dev = &pdev->dev;
>>      struct resource *mem;
>>      struct rproc *rproc;
>> @@ -181,6 +243,7 @@ static int ingenic_rproc_probe(struct 
>> platform_device *pdev)
>>
>>      vpu = rproc->priv;
>>      vpu->dev = &pdev->dev;
>> +    vpu->soc_info = id->data;
>
> Use of_device_get_match_data(dev).
>
> Then you can get rid of the "id" variable, and you won't have to move 
> the "ingenic_rproc_of_matches" array.


Sure, I will try.


>
>>      platform_set_drvdata(pdev, vpu);
>>
>>      mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "aux");
>> @@ -190,9 +253,13 @@ static int ingenic_rproc_probe(struct 
>> platform_device *pdev)
>>          return PTR_ERR(vpu->aux_base);
>>      }
>>
>> -    for (i = 0; i < ARRAY_SIZE(vpu_mem_map); i++) {
>> +    vpu->mem_info = kzalloc(sizeof(struct vpu_mem_info) * 
>> vpu->soc_info->num_mems, GFP_KERNEL);
>
> You are leaking memory here.
>
> Also, why not just fix the current mem_info array size to 3? That 
> sounds way simpler.


Sure.


>
>> +    if (!vpu->mem_info)
>> +        return -ENOMEM;
>> +
>> +    for (i = 0; i < vpu->soc_info->num_mems; i++) {
>>          mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>> -                           vpu_mem_map[i].name);
>> + vpu->soc_info->mem_map[i].name);
>>
>>          vpu->mem_info[i].base = devm_ioremap_resource(dev, mem);
>>          if (IS_ERR(vpu->mem_info[i].base)) {
>> @@ -202,13 +269,19 @@ static int ingenic_rproc_probe(struct 
>> platform_device *pdev)
>>          }
>>
>>          vpu->mem_info[i].len = resource_size(mem);
>> -        vpu->mem_info[i].map = &vpu_mem_map[i];
>> +        vpu->mem_info[i].map = &vpu->soc_info->mem_map[i];
>>      }
>>
>> +    vpu->clks = kzalloc(sizeof(struct clk_bulk_data) * 
>> vpu->soc_info->num_clks, GFP_KERNEL);
>
> Same here, the "clks" array is already size 2, so it won't be a 
> problem if you have only one clock. No need to alloc "clks" dynamically.


Sure.


>
>> +    if (!vpu->clks)
>> +        return -ENOMEM;
>> +
>>      vpu->clks[0].id = "vpu";
>> -    vpu->clks[1].id = "aux";
>>
>> -    ret = devm_clk_bulk_get(dev, ARRAY_SIZE(vpu->clks), vpu->clks);
>> +    if (vpu->soc_info->version == ID_JZ4770)
>> +        vpu->clks[1].id = "aux";
>> +
>> +    ret = devm_clk_bulk_get(dev, vpu->soc_info->num_clks, vpu->clks);
>>      if (ret) {
>>          dev_err(dev, "Failed to get clocks\n");
>>          return ret;
>> @@ -235,12 +308,6 @@ static int ingenic_rproc_probe(struct 
>> platform_device *pdev)
>>      return 0;
>>  }
>>
>> -static const struct of_device_id ingenic_rproc_of_matches[] = {
>> -    { .compatible = "ingenic,jz4770-vpu-rproc", },
>> -    {}
>> -};
>> -MODULE_DEVICE_TABLE(of, ingenic_rproc_of_matches);
>
> As I wrote above - you don't need to move this.


Sure.


Thanks and best regards!


>
> Cheers,
> -Paul
>
>> -
>>  static struct platform_driver ingenic_rproc_driver = {
>>      .probe = ingenic_rproc_probe,
>>      .driver = {
>> -- 
>> 2.7.4
>>
>
kernel test robot July 24, 2021, 7:30 p.m. UTC | #3
Hi "周琰杰,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on v5.14-rc2 next-20210723]
[cannot apply to remoteproc/for-next rpmsg/for-next]
[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/0day-ci/linux/commits/Zhou-Yanjie/Add-VPU-support-for-new-Ingenic-SoCs/20210724-171238
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: sparc-allyesconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 10.3.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/0day-ci/linux/commit/288ce023c75dca6dd232f6166be5a07d5532aad7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Zhou-Yanjie/Add-VPU-support-for-new-Ingenic-SoCs/20210724-171238
        git checkout 288ce023c75dca6dd232f6166be5a07d5532aad7
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross ARCH=sparc 

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

All warnings (new ones prefixed by >>):

   drivers/remoteproc/ingenic_rproc.c: In function 'ingenic_rproc_probe':
   drivers/remoteproc/ingenic_rproc.c:256:18: error: implicit declaration of function 'kzalloc'; did you mean 'kvzalloc'? [-Werror=implicit-function-declaration]
     256 |  vpu->mem_info = kzalloc(sizeof(struct vpu_mem_info) * vpu->soc_info->num_mems, GFP_KERNEL);
         |                  ^~~~~~~
         |                  kvzalloc
>> drivers/remoteproc/ingenic_rproc.c:256:16: warning: assignment to 'struct vpu_mem_info *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     256 |  vpu->mem_info = kzalloc(sizeof(struct vpu_mem_info) * vpu->soc_info->num_mems, GFP_KERNEL);
         |                ^
>> drivers/remoteproc/ingenic_rproc.c:275:12: warning: assignment to 'struct clk_bulk_data *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     275 |  vpu->clks = kzalloc(sizeof(struct clk_bulk_data) * vpu->soc_info->num_clks, GFP_KERNEL);
         |            ^
   cc1: some warnings being treated as errors


vim +256 drivers/remoteproc/ingenic_rproc.c

   226	
   227	static int ingenic_rproc_probe(struct platform_device *pdev)
   228	{
   229		const struct of_device_id *id = of_match_node(ingenic_rproc_of_matches, pdev->dev.of_node);
   230		struct device *dev = &pdev->dev;
   231		struct resource *mem;
   232		struct rproc *rproc;
   233		struct vpu *vpu;
   234		unsigned int i;
   235		int ret;
   236	
   237		rproc = devm_rproc_alloc(dev, "ingenic-vpu",
   238					 &ingenic_rproc_ops, NULL, sizeof(*vpu));
   239		if (!rproc)
   240			return -ENOMEM;
   241	
   242		rproc->auto_boot = auto_boot;
   243	
   244		vpu = rproc->priv;
   245		vpu->dev = &pdev->dev;
   246		vpu->soc_info = id->data;
   247		platform_set_drvdata(pdev, vpu);
   248	
   249		mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "aux");
   250		vpu->aux_base = devm_ioremap_resource(dev, mem);
   251		if (IS_ERR(vpu->aux_base)) {
   252			dev_err(dev, "Failed to ioremap\n");
   253			return PTR_ERR(vpu->aux_base);
   254		}
   255	
 > 256		vpu->mem_info = kzalloc(sizeof(struct vpu_mem_info) * vpu->soc_info->num_mems, GFP_KERNEL);
   257		if (!vpu->mem_info)
   258			return -ENOMEM;
   259	
   260		for (i = 0; i < vpu->soc_info->num_mems; i++) {
   261			mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
   262							   vpu->soc_info->mem_map[i].name);
   263	
   264			vpu->mem_info[i].base = devm_ioremap_resource(dev, mem);
   265			if (IS_ERR(vpu->mem_info[i].base)) {
   266				ret = PTR_ERR(vpu->mem_info[i].base);
   267				dev_err(dev, "Failed to ioremap\n");
   268				return ret;
   269			}
   270	
   271			vpu->mem_info[i].len = resource_size(mem);
   272			vpu->mem_info[i].map = &vpu->soc_info->mem_map[i];
   273		}
   274	
 > 275		vpu->clks = kzalloc(sizeof(struct clk_bulk_data) * vpu->soc_info->num_clks, GFP_KERNEL);
   276		if (!vpu->clks)
   277			return -ENOMEM;
   278	
   279		vpu->clks[0].id = "vpu";
   280	
   281		if (vpu->soc_info->version == ID_JZ4770)
   282			vpu->clks[1].id = "aux";
   283	
   284		ret = devm_clk_bulk_get(dev, vpu->soc_info->num_clks, vpu->clks);
   285		if (ret) {
   286			dev_err(dev, "Failed to get clocks\n");
   287			return ret;
   288		}
   289	
   290		vpu->irq = platform_get_irq(pdev, 0);
   291		if (vpu->irq < 0)
   292			return vpu->irq;
   293	
   294		ret = devm_request_irq(dev, vpu->irq, vpu_interrupt, 0, "VPU", rproc);
   295		if (ret < 0) {
   296			dev_err(dev, "Failed to request IRQ\n");
   297			return ret;
   298		}
   299	
   300		disable_irq(vpu->irq);
   301	
   302		ret = devm_rproc_add(dev, rproc);
   303		if (ret) {
   304			dev_err(dev, "Failed to register remote processor\n");
   305			return ret;
   306		}
   307	
   308		return 0;
   309	}
   310	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot July 25, 2021, 1:26 a.m. UTC | #4
Hi "周琰杰,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on v5.14-rc2 next-20210723]
[cannot apply to remoteproc/for-next rpmsg/for-next]
[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/0day-ci/linux/commits/Zhou-Yanjie/Add-VPU-support-for-new-Ingenic-SoCs/20210724-171238
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: sparc-allyesconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 10.3.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/0day-ci/linux/commit/288ce023c75dca6dd232f6166be5a07d5532aad7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Zhou-Yanjie/Add-VPU-support-for-new-Ingenic-SoCs/20210724-171238
        git checkout 288ce023c75dca6dd232f6166be5a07d5532aad7
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross ARCH=sparc 

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/remoteproc/ingenic_rproc.c: In function 'ingenic_rproc_probe':
>> drivers/remoteproc/ingenic_rproc.c:256:18: error: implicit declaration of function 'kzalloc'; did you mean 'kvzalloc'? [-Werror=implicit-function-declaration]
     256 |  vpu->mem_info = kzalloc(sizeof(struct vpu_mem_info) * vpu->soc_info->num_mems, GFP_KERNEL);
         |                  ^~~~~~~
         |                  kvzalloc
   drivers/remoteproc/ingenic_rproc.c:256:16: warning: assignment to 'struct vpu_mem_info *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     256 |  vpu->mem_info = kzalloc(sizeof(struct vpu_mem_info) * vpu->soc_info->num_mems, GFP_KERNEL);
         |                ^
   drivers/remoteproc/ingenic_rproc.c:275:12: warning: assignment to 'struct clk_bulk_data *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     275 |  vpu->clks = kzalloc(sizeof(struct clk_bulk_data) * vpu->soc_info->num_clks, GFP_KERNEL);
         |            ^
   cc1: some warnings being treated as errors


vim +256 drivers/remoteproc/ingenic_rproc.c

   226	
   227	static int ingenic_rproc_probe(struct platform_device *pdev)
   228	{
   229		const struct of_device_id *id = of_match_node(ingenic_rproc_of_matches, pdev->dev.of_node);
   230		struct device *dev = &pdev->dev;
   231		struct resource *mem;
   232		struct rproc *rproc;
   233		struct vpu *vpu;
   234		unsigned int i;
   235		int ret;
   236	
   237		rproc = devm_rproc_alloc(dev, "ingenic-vpu",
   238					 &ingenic_rproc_ops, NULL, sizeof(*vpu));
   239		if (!rproc)
   240			return -ENOMEM;
   241	
   242		rproc->auto_boot = auto_boot;
   243	
   244		vpu = rproc->priv;
   245		vpu->dev = &pdev->dev;
   246		vpu->soc_info = id->data;
   247		platform_set_drvdata(pdev, vpu);
   248	
   249		mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "aux");
   250		vpu->aux_base = devm_ioremap_resource(dev, mem);
   251		if (IS_ERR(vpu->aux_base)) {
   252			dev_err(dev, "Failed to ioremap\n");
   253			return PTR_ERR(vpu->aux_base);
   254		}
   255	
 > 256		vpu->mem_info = kzalloc(sizeof(struct vpu_mem_info) * vpu->soc_info->num_mems, GFP_KERNEL);
   257		if (!vpu->mem_info)
   258			return -ENOMEM;
   259	
   260		for (i = 0; i < vpu->soc_info->num_mems; i++) {
   261			mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
   262							   vpu->soc_info->mem_map[i].name);
   263	
   264			vpu->mem_info[i].base = devm_ioremap_resource(dev, mem);
   265			if (IS_ERR(vpu->mem_info[i].base)) {
   266				ret = PTR_ERR(vpu->mem_info[i].base);
   267				dev_err(dev, "Failed to ioremap\n");
   268				return ret;
   269			}
   270	
   271			vpu->mem_info[i].len = resource_size(mem);
   272			vpu->mem_info[i].map = &vpu->soc_info->mem_map[i];
   273		}
   274	
   275		vpu->clks = kzalloc(sizeof(struct clk_bulk_data) * vpu->soc_info->num_clks, GFP_KERNEL);
   276		if (!vpu->clks)
   277			return -ENOMEM;
   278	
   279		vpu->clks[0].id = "vpu";
   280	
   281		if (vpu->soc_info->version == ID_JZ4770)
   282			vpu->clks[1].id = "aux";
   283	
   284		ret = devm_clk_bulk_get(dev, vpu->soc_info->num_clks, vpu->clks);
   285		if (ret) {
   286			dev_err(dev, "Failed to get clocks\n");
   287			return ret;
   288		}
   289	
   290		vpu->irq = platform_get_irq(pdev, 0);
   291		if (vpu->irq < 0)
   292			return vpu->irq;
   293	
   294		ret = devm_request_irq(dev, vpu->irq, vpu_interrupt, 0, "VPU", rproc);
   295		if (ret < 0) {
   296			dev_err(dev, "Failed to request IRQ\n");
   297			return ret;
   298		}
   299	
   300		disable_irq(vpu->irq);
   301	
   302		ret = devm_rproc_add(dev, rproc);
   303		if (ret) {
   304			dev_err(dev, "Failed to register remote processor\n");
   305			return ret;
   306		}
   307	
   308		return 0;
   309	}
   310	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/remoteproc/ingenic_rproc.c b/drivers/remoteproc/ingenic_rproc.c
index a356738..6a2e864 100644
--- a/drivers/remoteproc/ingenic_rproc.c
+++ b/drivers/remoteproc/ingenic_rproc.c
@@ -2,6 +2,7 @@ 
 /*
  * Ingenic JZ47xx remoteproc driver
  * Copyright 2019, Paul Cercueil <paul@crapouillou.net>
+ * Copyright 2021, 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
  */
 
 #include <linux/bitops.h>
@@ -17,7 +18,7 @@ 
 
 #define REG_AUX_CTRL		0x0
 #define REG_AUX_MSG_ACK		0x10
-#define REG_AUX_MSG		0x14
+#define REG_AUX_MSG			0x14
 #define REG_CORE_MSG_ACK	0x18
 #define REG_CORE_MSG		0x1C
 
@@ -32,6 +33,20 @@  module_param(auto_boot, bool, 0400);
 MODULE_PARM_DESC(auto_boot,
 		 "Auto-boot the remote processor [default=false]");
 
+enum ingenic_vpu_version {
+	ID_JZ4760,
+	ID_JZ4770,
+	ID_JZ4775,
+};
+
+struct ingenic_soc_info {
+	enum ingenic_vpu_version version;
+	const struct vpu_mem_map *mem_map;
+
+	unsigned int num_clks;
+	unsigned int num_mems;
+};
+
 struct vpu_mem_map {
 	const char *name;
 	unsigned int da;
@@ -43,26 +58,21 @@  struct vpu_mem_info {
 	void __iomem *base;
 };
 
-static const struct vpu_mem_map vpu_mem_map[] = {
-	{ "tcsm0", 0x132b0000 },
-	{ "tcsm1", 0xf4000000 },
-	{ "sram",  0x132f0000 },
-};
-
 /**
  * struct vpu - Ingenic VPU remoteproc private structure
  * @irq: interrupt number
  * @clks: pointers to the VPU and AUX clocks
  * @aux_base: raw pointer to the AUX interface registers
- * @mem_info: array of struct vpu_mem_info, which contain the mapping info of
+ * @mem_info: pointers to the struct vpu_mem_info, which contain the mapping info of
  *            each of the external memories
  * @dev: private pointer to the device
  */
 struct vpu {
 	int irq;
-	struct clk_bulk_data clks[2];
 	void __iomem *aux_base;
-	struct vpu_mem_info mem_info[ARRAY_SIZE(vpu_mem_map)];
+	const struct ingenic_soc_info *soc_info;
+	struct clk_bulk_data *clks;
+	struct vpu_mem_info *mem_info;
 	struct device *dev;
 };
 
@@ -72,7 +82,7 @@  static int ingenic_rproc_prepare(struct rproc *rproc)
 	int ret;
 
 	/* The clocks must be enabled for the firmware to be loaded in TCSM */
-	ret = clk_bulk_prepare_enable(ARRAY_SIZE(vpu->clks), vpu->clks);
+	ret = clk_bulk_prepare_enable(vpu->soc_info->num_clks, vpu->clks);
 	if (ret)
 		dev_err(vpu->dev, "Unable to start clocks: %d\n", ret);
 
@@ -83,7 +93,7 @@  static int ingenic_rproc_unprepare(struct rproc *rproc)
 {
 	struct vpu *vpu = rproc->priv;
 
-	clk_bulk_disable_unprepare(ARRAY_SIZE(vpu->clks), vpu->clks);
+	clk_bulk_disable_unprepare(vpu->soc_info->num_clks, vpu->clks);
 
 	return 0;
 }
@@ -127,7 +137,7 @@  static void *ingenic_rproc_da_to_va(struct rproc *rproc, u64 da, size_t len, boo
 	void __iomem *va = NULL;
 	unsigned int i;
 
-	for (i = 0; i < ARRAY_SIZE(vpu_mem_map); i++) {
+	for (i = 0; i < vpu->soc_info->num_mems; i++) {
 		const struct vpu_mem_info *info = &vpu->mem_info[i];
 		const struct vpu_mem_map *map = info->map;
 
@@ -163,8 +173,60 @@  static irqreturn_t vpu_interrupt(int irq, void *data)
 	return rproc_vq_interrupt(rproc, vring);
 }
 
+static const struct vpu_mem_map jz4760_vpu_mem_map[] = {
+	{ "tcsm0", 0x132b0000 },
+	{ "tcsm1", 0xf4000000 },
+	{ "sram",  0x132d0000 },
+};
+
+static const struct vpu_mem_map jz4770_vpu_mem_map[] = {
+	{ "tcsm0", 0x132b0000 },
+	{ "tcsm1", 0xf4000000 },
+	{ "sram",  0x132f0000 },
+};
+
+static const struct vpu_mem_map jz4775_vpu_mem_map[] = {
+	{ "tcsm",  0xf4000000 },
+	{ "sram",  0x132f0000 },
+};
+
+static const struct ingenic_soc_info jz4760_soc_info = {
+	.version = ID_JZ4760,
+	.mem_map = jz4760_vpu_mem_map,
+
+	.num_clks = 2,
+	.num_mems = 3,
+};
+
+static const struct ingenic_soc_info jz4770_soc_info = {
+	.version = ID_JZ4770,
+	.mem_map = jz4770_vpu_mem_map,
+
+	.num_clks = 2,
+	.num_mems = 3,
+};
+
+static const struct ingenic_soc_info jz4775_soc_info = {
+	.version = ID_JZ4775,
+	.mem_map = jz4775_vpu_mem_map,
+
+	.num_clks = 1,
+	.num_mems = 2,
+};
+
+static const struct of_device_id ingenic_rproc_of_matches[] = {
+	{ .compatible = "ingenic,jz4760-vpu-rproc", .data = &jz4760_soc_info },
+	{ .compatible = "ingenic,jz4760b-vpu-rproc", .data = &jz4760_soc_info },
+	{ .compatible = "ingenic,jz4770-vpu-rproc", .data = &jz4770_soc_info },
+	{ .compatible = "ingenic,jz4775-vpu-rproc", .data = &jz4775_soc_info },
+	{ .compatible = "ingenic,jz4780-vpu-rproc", .data = &jz4775_soc_info },
+	{}
+};
+MODULE_DEVICE_TABLE(of, ingenic_rproc_of_matches);
+
 static int ingenic_rproc_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *id = of_match_node(ingenic_rproc_of_matches, pdev->dev.of_node);
 	struct device *dev = &pdev->dev;
 	struct resource *mem;
 	struct rproc *rproc;
@@ -181,6 +243,7 @@  static int ingenic_rproc_probe(struct platform_device *pdev)
 
 	vpu = rproc->priv;
 	vpu->dev = &pdev->dev;
+	vpu->soc_info = id->data;
 	platform_set_drvdata(pdev, vpu);
 
 	mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "aux");
@@ -190,9 +253,13 @@  static int ingenic_rproc_probe(struct platform_device *pdev)
 		return PTR_ERR(vpu->aux_base);
 	}
 
-	for (i = 0; i < ARRAY_SIZE(vpu_mem_map); i++) {
+	vpu->mem_info = kzalloc(sizeof(struct vpu_mem_info) * vpu->soc_info->num_mems, GFP_KERNEL);
+	if (!vpu->mem_info)
+		return -ENOMEM;
+
+	for (i = 0; i < vpu->soc_info->num_mems; i++) {
 		mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-						   vpu_mem_map[i].name);
+						   vpu->soc_info->mem_map[i].name);
 
 		vpu->mem_info[i].base = devm_ioremap_resource(dev, mem);
 		if (IS_ERR(vpu->mem_info[i].base)) {
@@ -202,13 +269,19 @@  static int ingenic_rproc_probe(struct platform_device *pdev)
 		}
 
 		vpu->mem_info[i].len = resource_size(mem);
-		vpu->mem_info[i].map = &vpu_mem_map[i];
+		vpu->mem_info[i].map = &vpu->soc_info->mem_map[i];
 	}
 
+	vpu->clks = kzalloc(sizeof(struct clk_bulk_data) * vpu->soc_info->num_clks, GFP_KERNEL);
+	if (!vpu->clks)
+		return -ENOMEM;
+
 	vpu->clks[0].id = "vpu";
-	vpu->clks[1].id = "aux";
 
-	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(vpu->clks), vpu->clks);
+	if (vpu->soc_info->version == ID_JZ4770)
+		vpu->clks[1].id = "aux";
+
+	ret = devm_clk_bulk_get(dev, vpu->soc_info->num_clks, vpu->clks);
 	if (ret) {
 		dev_err(dev, "Failed to get clocks\n");
 		return ret;
@@ -235,12 +308,6 @@  static int ingenic_rproc_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id ingenic_rproc_of_matches[] = {
-	{ .compatible = "ingenic,jz4770-vpu-rproc", },
-	{}
-};
-MODULE_DEVICE_TABLE(of, ingenic_rproc_of_matches);
-
 static struct platform_driver ingenic_rproc_driver = {
 	.probe = ingenic_rproc_probe,
 	.driver = {