Re: [PATCH v6 04/12] firmware: ti_sci: Add RM mapping table for am654

From: Lokesh Vutla <lokeshvutla@ti.com>
To: Tony Lindgren <tony@atomide.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>, Nishanth Menon <nm@ti.com>,
	Santosh Shilimkar <ssantosh@kernel.org>,
	Rob Herring <robh+dt@kernel.org>, <jason@lakedaemon.net>,
	Linux ARM Mailing List <linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>,
	Device Tree Mailing List <devicetree@vger.kernel.org>,
	Sekhar Nori <nsekhar@ti.com>, Tero Kristo <t-kristo@ti.com>,
	Peter Ujfalusi <peter.ujfalusi@ti.com>,
	Grygorii Strashko <grygorii.strashko@ti.com>
Subject: Re: [PATCH v6 04/12] firmware: ti_sci: Add RM mapping table for am654
Date: Fri, 12 Apr 2019 09:38:05 +0530	[thread overview]
Message-ID: <49dd19bf-f5bd-e048-75ad-2d4cb708bcf9@ti.com> (raw)
In-Reply-To: <20190411145416.GP2839@atomide.com>

On 11/04/19 8:24 PM, Tony Lindgren wrote:
> Hi,
> 
> * Lokesh Vutla <lokeshvutla@ti.com> [190410 04:15]:
>> From: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt b/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>> index b56a02c10ae6..6f0cd31c1520 100644
>> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>> @@ -24,7 +24,8 @@ relationship between the TI-SCI parent node to the child node.
>>  
>>  Required properties:
>>  -------------------
>> -- compatible: should be "ti,k2g-sci"
>> +- compatible:	should be "ti,k2g-sci" for TI 66AK2G SoC
>> +		should be "ti,am654-sci" for for TI AM654 SoC
>>  - mbox-names:
>>  	"rx" - Mailbox corresponding to receive path
>>  	"tx" - Mailbox corresponding to transmit path
>> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
>> index d303f5a14da9..88e461498def 100644
>> --- a/drivers/firmware/ti_sci.c
>> +++ b/drivers/firmware/ti_sci.c
>> @@ -2297,10 +2297,33 @@ static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
>>  	/* Limited by MBOX_TX_QUEUE_LEN. K2G can handle upto 128 messages! */
>>  	.max_msgs = 20,
>>  	.max_msg_size = 64,
>> +	.rm_type_map = NULL,
>> +};
>> +
>> +static struct ti_sci_rm_type_map ti_sci_am654_rm_type_map[] = {
>> +	{.dev_id = 56, .type = 0x00b}, /* GIC_IRQ */
>> +	{.dev_id = 179, .type = 0x000}, /* MAIN_NAV_UDMASS_IA0 */
>> +	{.dev_id = 187, .type = 0x009}, /* MAIN_NAV_RA */
>> +	{.dev_id = 188, .type = 0x006}, /* MAIN_NAV_UDMAP */
>> +	{.dev_id = 194, .type = 0x007}, /* MCU_NAV_UDMAP */
>> +	{.dev_id = 195, .type = 0x00a}, /* MCU_NAV_RA */
>> +	{.dev_id = 0, .type = 0x000}, /* end of table */
>> +};
>> +
>> +/* Description for AM654 */
>> +static const struct ti_sci_desc ti_sci_pmmc_am654_desc = {
>> +	.default_host_id = 12,
>> +	/* Conservative duration */
>> +	.max_rx_timeout_ms = 10000,
>> +	/* Limited by MBOX_TX_QUEUE_LEN. K2G can handle upto 128 messages! */
>> +	.max_msgs = 20,
>> +	.max_msg_size = 60,
>> +	.rm_type_map = ti_sci_am654_rm_type_map,
>>  };
>>  
>>  static const struct of_device_id ti_sci_of_match[] = {
>>  	{.compatible = "ti,k2g-sci", .data = &ti_sci_pmmc_k2g_desc},
>> +	{.compatible = "ti,am654-sci", .data = &ti_sci_pmmc_am654_desc},
>>  	{ /* Sentinel */ },
>>  };
>>  MODULE_DEVICE_TABLE(of, ti_sci_of_match);
> 
> Great, this approach with mapping table in the driver based on
> the compatible looks good to me and avoids stuffing the IDs
> into device tree:
> 
> Acked-by: Tony Lindgren <tony@atomide.com>
> 

Thanks, but I don't think you understood what the patch is actually doing.
Please look at the rest of the series on how this table is being used.

Thanks and regards,
Lokesh