xhci: tegra: Parameterize mailbox register addresses

Message ID	20190902082127.17963-1-jckuo@nvidia.com
State	Superseded
Headers	show Return-Path: <linux-tegra-owner@vger.kernel.org> TLS: TLSv1.2, DES-CBC3-SHA) id <B5d6cd10d0000>; Mon, 02 Sep 2019 01:21:33 -0700 From: JC Kuo <jckuo@nvidia.com> To: <gregkh@linuxfoundation.org>, <thierry.reding@gmail.com>, <jonathanh@nvidia.com> CC: <linux-tegra@vger.kernel.org>, <linux-usb@vger.kernel.org>, JC Kuo <jckuo@nvidia.com> Subject: [PATCH] xhci: tegra: Parameterize mailbox register addresses Date: Mon, 2 Sep 2019 16:21:27 +0800 Message-ID: <20190902082127.17963-1-jckuo@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-tegra-owner@vger.kernel.org Precedence: bulk
Series	xhci: tegra: Parameterize mailbox register addresses \| expand xhci: tegra: Parameterize mailbox register addresses

Message ID

20190902082127.17963-1-jckuo@nvidia.com

State

Superseded

Headers

From: JC Kuo <jckuo@nvidia.com>
To: <gregkh@linuxfoundation.org>, <thierry.reding@gmail.com>,
	<jonathanh@nvidia.com>
CC: <linux-tegra@vger.kernel.org>, <linux-usb@vger.kernel.org>,
	JC Kuo <jckuo@nvidia.com>
Subject: [PATCH] xhci: tegra: Parameterize mailbox register addresses
Date: Mon, 2 Sep 2019 16:21:27 +0800
Message-ID: <20190902082127.17963-1-jckuo@nvidia.com>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-tegra-owner@vger.kernel.org
Precedence: bulk

Series

xhci: tegra: Parameterize mailbox register addresses | expand

Commit Message

JC Kuo Sept. 2, 2019, 8:21 a.m. UTC

Tegra194 XUSB host controller has rearranged mailbox registers. This
commit makes mailbox registers address a part of "soc" data so that
xhci-tegra driver can be used for Tegra194.

Signed-off-by: JC Kuo <jckuo@nvidia.com>
---
 drivers/usb/host/xhci-tegra.c | 58 +++++++++++++++++++++++++----------
 1 file changed, 42 insertions(+), 16 deletions(-)

Comments

Greg Kroah-Hartman Sept. 3, 2019, 1:58 p.m. UTC | #1

On Mon, Sep 02, 2019 at 04:21:27PM +0800, JC Kuo wrote:
> Tegra194 XUSB host controller has rearranged mailbox registers. This
> commit makes mailbox registers address a part of "soc" data so that
> xhci-tegra driver can be used for Tegra194.
> 
> Signed-off-by: JC Kuo <jckuo@nvidia.com>
> ---
>  drivers/usb/host/xhci-tegra.c | 58 +++++++++++++++++++++++++----------
>  1 file changed, 42 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index dafc65911fc0..247b08ca49ee 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -42,19 +42,18 @@
>  #define XUSB_CFG_CSB_BASE_ADDR			0x800
>  
>  /* FPCI mailbox registers */
> -#define XUSB_CFG_ARU_MBOX_CMD			0x0e4
> +/* XUSB_CFG_ARU_MBOX_CMD */
>  #define  MBOX_DEST_FALC				BIT(27)
>  #define  MBOX_DEST_PME				BIT(28)
>  #define  MBOX_DEST_SMI				BIT(29)
>  #define  MBOX_DEST_XHCI				BIT(30)
>  #define  MBOX_INT_EN				BIT(31)
> -#define XUSB_CFG_ARU_MBOX_DATA_IN		0x0e8
> +/* XUSB_CFG_ARU_MBOX_DATA_IN and XUSB_CFG_ARU_MBOX_DATA_OUT */
>  #define  CMD_DATA_SHIFT				0
>  #define  CMD_DATA_MASK				0xffffff
>  #define  CMD_TYPE_SHIFT				24
>  #define  CMD_TYPE_MASK				0xff
> -#define XUSB_CFG_ARU_MBOX_DATA_OUT		0x0ec
> -#define XUSB_CFG_ARU_MBOX_OWNER			0x0f0
> +/* XUSB_CFG_ARU_MBOX_OWNER */
>  #define  MBOX_OWNER_NONE			0
>  #define  MBOX_OWNER_FW				1
>  #define  MBOX_OWNER_SW				2
> @@ -146,6 +145,13 @@ struct tegra_xusb_phy_type {
>  	unsigned int num;
>  };
>  
> +struct tega_xusb_mbox_regs {
> +	unsigned int cmd;
> +	unsigned int data_in;
> +	unsigned int data_out;
> +	unsigned int owner;

Shouldn't these all be u8 values?

> +};
> +
>  struct tegra_xusb_soc {
>  	const char *firmware;
>  	const char * const *supply_names;
> @@ -160,6 +166,8 @@ struct tegra_xusb_soc {
>  		} usb2, ulpi, hsic, usb3;
>  	} ports;
>  
> +	struct tega_xusb_mbox_regs mbox;
> +
>  	bool scale_ss_clock;
>  	bool has_ipfs;
>  };
> @@ -395,15 +403,15 @@ static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
>  	 * ACK/NAK messages.
>  	 */
>  	if (!(msg->cmd == MBOX_CMD_ACK || msg->cmd == MBOX_CMD_NAK)) {
> -		value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_OWNER);
> +		value = fpci_readl(tegra, tegra->soc->mbox.owner);
>  		if (value != MBOX_OWNER_NONE) {
>  			dev_err(tegra->dev, "mailbox is busy\n");
>  			return -EBUSY;
>  		}
>  
> -		fpci_writel(tegra, MBOX_OWNER_SW, XUSB_CFG_ARU_MBOX_OWNER);
> +		fpci_writel(tegra, MBOX_OWNER_SW, tegra->soc->mbox.owner);
>  
> -		value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_OWNER);
> +		value = fpci_readl(tegra, tegra->soc->mbox.owner);
>  		if (value != MBOX_OWNER_SW) {
>  			dev_err(tegra->dev, "failed to acquire mailbox\n");
>  			return -EBUSY;
> @@ -413,17 +421,17 @@ static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
>  	}
>  
>  	value = tegra_xusb_mbox_pack(msg);
> -	fpci_writel(tegra, value, XUSB_CFG_ARU_MBOX_DATA_IN);
> +	fpci_writel(tegra, value, tegra->soc->mbox.data_in);
>  
> -	value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_CMD);
> +	value = fpci_readl(tegra, tegra->soc->mbox.cmd);
>  	value |= MBOX_INT_EN | MBOX_DEST_FALC;
> -	fpci_writel(tegra, value, XUSB_CFG_ARU_MBOX_CMD);
> +	fpci_writel(tegra, value, tegra->soc->mbox.cmd);
>  
>  	if (wait_for_idle) {
>  		unsigned long timeout = jiffies + msecs_to_jiffies(250);
>  
>  		while (time_before(jiffies, timeout)) {
> -			value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_OWNER);
> +			value = fpci_readl(tegra, tegra->soc->mbox.owner);
>  			if (value == MBOX_OWNER_NONE)
>  				break;
>  
> @@ -431,7 +439,7 @@ static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
>  		}
>  
>  		if (time_after(jiffies, timeout))
> -			value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_OWNER);
> +			value = fpci_readl(tegra, tegra->soc->mbox.owner);
>  
>  		if (value != MBOX_OWNER_NONE)
>  			return -ETIMEDOUT;
> @@ -598,16 +606,16 @@ static irqreturn_t tegra_xusb_mbox_thread(int irq, void *data)
>  
>  	mutex_lock(&tegra->lock);
>  
> -	value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_DATA_OUT);
> +	value = fpci_readl(tegra, tegra->soc->mbox.data_out);
>  	tegra_xusb_mbox_unpack(&msg, value);
>  
> -	value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_CMD);
> +	value = fpci_readl(tegra, tegra->soc->mbox.cmd);
>  	value &= ~MBOX_DEST_SMI;
> -	fpci_writel(tegra, value, XUSB_CFG_ARU_MBOX_CMD);
> +	fpci_writel(tegra, value, tegra->soc->mbox.cmd);
>  
>  	/* clear mailbox owner if no ACK/NAK is required */
>  	if (!tegra_xusb_mbox_cmd_requires_ack(msg.cmd))
> -		fpci_writel(tegra, MBOX_OWNER_NONE, XUSB_CFG_ARU_MBOX_OWNER);
> +		fpci_writel(tegra, MBOX_OWNER_NONE, tegra->soc->mbox.owner);
>  
>  	tegra_xusb_mbox_handle(tegra, &msg);
>  
> @@ -1365,6 +1373,12 @@ static const struct tegra_xusb_soc tegra124_soc = {
>  	},
>  	.scale_ss_clock = true,
>  	.has_ipfs = true,
> +	.mbox = {
> +		.cmd = 0xe4,
> +		.data_in = 0xe8,
> +		.data_out = 0xec,
> +		.owner = 0xf0,
> +	},
>  };
>  MODULE_FIRMWARE("nvidia/tegra124/xusb.bin");
>  
> @@ -1397,6 +1411,12 @@ static const struct tegra_xusb_soc tegra210_soc = {
>  	},
>  	.scale_ss_clock = false,
>  	.has_ipfs = true,
> +	.mbox = {
> +		.cmd = 0xe4,
> +		.data_in = 0xe8,
> +		.data_out = 0xec,
> +		.owner = 0xf0,
> +	},
>  };
>  MODULE_FIRMWARE("nvidia/tegra210/xusb.bin");
>  
> @@ -1422,6 +1442,12 @@ static const struct tegra_xusb_soc tegra186_soc = {
>  	},
>  	.scale_ss_clock = false,
>  	.has_ipfs = false,
> +	.mbox = {
> +		.cmd = 0xe4,
> +		.data_in = 0xe8,
> +		.data_out = 0xec,
> +		.owner = 0xf0,
> +	},

This did not change any existing functionality, is there a follow-on
patch somewhere that takes advantage of this change to enable different
hardware?  Otherwise this doesn't seem worth it.

thanks,

greg k-h

JC Kuo Sept. 4, 2019, 1:43 a.m. UTC | #2

On 9/3/19 9:58 PM, Greg KH wrote:
> On Mon, Sep 02, 2019 at 04:21:27PM +0800, JC Kuo wrote:
>> Tegra194 XUSB host controller has rearranged mailbox registers. This
>> commit makes mailbox registers address a part of "soc" data so that
>> xhci-tegra driver can be used for Tegra194.
>>
>> Signed-off-by: JC Kuo <jckuo@nvidia.com>
>> ---
>>  drivers/usb/host/xhci-tegra.c | 58 +++++++++++++++++++++++++----------
>>  1 file changed, 42 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
>> index dafc65911fc0..247b08ca49ee 100644
>> --- a/drivers/usb/host/xhci-tegra.c
>> +++ b/drivers/usb/host/xhci-tegra.c
>> @@ -42,19 +42,18 @@
>>  #define XUSB_CFG_CSB_BASE_ADDR			0x800
>>  
>>  /* FPCI mailbox registers */
>> -#define XUSB_CFG_ARU_MBOX_CMD			0x0e4
>> +/* XUSB_CFG_ARU_MBOX_CMD */
>>  #define  MBOX_DEST_FALC				BIT(27)
>>  #define  MBOX_DEST_PME				BIT(28)
>>  #define  MBOX_DEST_SMI				BIT(29)
>>  #define  MBOX_DEST_XHCI				BIT(30)
>>  #define  MBOX_INT_EN				BIT(31)
>> -#define XUSB_CFG_ARU_MBOX_DATA_IN		0x0e8
>> +/* XUSB_CFG_ARU_MBOX_DATA_IN and XUSB_CFG_ARU_MBOX_DATA_OUT */
>>  #define  CMD_DATA_SHIFT				0
>>  #define  CMD_DATA_MASK				0xffffff
>>  #define  CMD_TYPE_SHIFT				24
>>  #define  CMD_TYPE_MASK				0xff
>> -#define XUSB_CFG_ARU_MBOX_DATA_OUT		0x0ec
>> -#define XUSB_CFG_ARU_MBOX_OWNER			0x0f0
>> +/* XUSB_CFG_ARU_MBOX_OWNER */
>>  #define  MBOX_OWNER_NONE			0
>>  #define  MBOX_OWNER_FW				1
>>  #define  MBOX_OWNER_SW				2
>> @@ -146,6 +145,13 @@ struct tegra_xusb_phy_type {
>>  	unsigned int num;
>>  };
>>  
>> +struct tega_xusb_mbox_regs {
>> +	unsigned int cmd;
>> +	unsigned int data_in;
>> +	unsigned int data_out;
>> +	unsigned int owner;
> 
> Shouldn't these all be u8 values?
> 
These data members represent register offset in Tegra XUSB FPCI area. Size of
FPCI area is 0x1000, so it is possible for future Tegra XUSB to have mailbox
registers allocated to somewhere > 0x100.

>> +};
>> +
>>  struct tegra_xusb_soc {
>>  	const char *firmware;
>>  	const char * const *supply_names;
>> @@ -160,6 +166,8 @@ struct tegra_xusb_soc {
>>  		} usb2, ulpi, hsic, usb3;
>>  	} ports;
>>  
>> +	struct tega_xusb_mbox_regs mbox;
>> +
>>  	bool scale_ss_clock;
>>  	bool has_ipfs;
>>  };
>> @@ -395,15 +403,15 @@ static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
>>  	 * ACK/NAK messages.
>>  	 */
>>  	if (!(msg->cmd == MBOX_CMD_ACK || msg->cmd == MBOX_CMD_NAK)) {
>> -		value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_OWNER);
>> +		value = fpci_readl(tegra, tegra->soc->mbox.owner);
>>  		if (value != MBOX_OWNER_NONE) {
>>  			dev_err(tegra->dev, "mailbox is busy\n");
>>  			return -EBUSY;
>>  		}
>>  
>> -		fpci_writel(tegra, MBOX_OWNER_SW, XUSB_CFG_ARU_MBOX_OWNER);
>> +		fpci_writel(tegra, MBOX_OWNER_SW, tegra->soc->mbox.owner);
>>  
>> -		value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_OWNER);
>> +		value = fpci_readl(tegra, tegra->soc->mbox.owner);
>>  		if (value != MBOX_OWNER_SW) {
>>  			dev_err(tegra->dev, "failed to acquire mailbox\n");
>>  			return -EBUSY;
>> @@ -413,17 +421,17 @@ static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
>>  	}
>>  
>>  	value = tegra_xusb_mbox_pack(msg);
>> -	fpci_writel(tegra, value, XUSB_CFG_ARU_MBOX_DATA_IN);
>> +	fpci_writel(tegra, value, tegra->soc->mbox.data_in);
>>  
>> -	value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_CMD);
>> +	value = fpci_readl(tegra, tegra->soc->mbox.cmd);
>>  	value |= MBOX_INT_EN | MBOX_DEST_FALC;
>> -	fpci_writel(tegra, value, XUSB_CFG_ARU_MBOX_CMD);
>> +	fpci_writel(tegra, value, tegra->soc->mbox.cmd);
>>  
>>  	if (wait_for_idle) {
>>  		unsigned long timeout = jiffies + msecs_to_jiffies(250);
>>  
>>  		while (time_before(jiffies, timeout)) {
>> -			value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_OWNER);
>> +			value = fpci_readl(tegra, tegra->soc->mbox.owner);
>>  			if (value == MBOX_OWNER_NONE)
>>  				break;
>>  
>> @@ -431,7 +439,7 @@ static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
>>  		}
>>  
>>  		if (time_after(jiffies, timeout))
>> -			value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_OWNER);
>> +			value = fpci_readl(tegra, tegra->soc->mbox.owner);
>>  
>>  		if (value != MBOX_OWNER_NONE)
>>  			return -ETIMEDOUT;
>> @@ -598,16 +606,16 @@ static irqreturn_t tegra_xusb_mbox_thread(int irq, void *data)
>>  
>>  	mutex_lock(&tegra->lock);
>>  
>> -	value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_DATA_OUT);
>> +	value = fpci_readl(tegra, tegra->soc->mbox.data_out);
>>  	tegra_xusb_mbox_unpack(&msg, value);
>>  
>> -	value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_CMD);
>> +	value = fpci_readl(tegra, tegra->soc->mbox.cmd);
>>  	value &= ~MBOX_DEST_SMI;
>> -	fpci_writel(tegra, value, XUSB_CFG_ARU_MBOX_CMD);
>> +	fpci_writel(tegra, value, tegra->soc->mbox.cmd);
>>  
>>  	/* clear mailbox owner if no ACK/NAK is required */
>>  	if (!tegra_xusb_mbox_cmd_requires_ack(msg.cmd))
>> -		fpci_writel(tegra, MBOX_OWNER_NONE, XUSB_CFG_ARU_MBOX_OWNER);
>> +		fpci_writel(tegra, MBOX_OWNER_NONE, tegra->soc->mbox.owner);
>>  
>>  	tegra_xusb_mbox_handle(tegra, &msg);
>>  
>> @@ -1365,6 +1373,12 @@ static const struct tegra_xusb_soc tegra124_soc = {
>>  	},
>>  	.scale_ss_clock = true,
>>  	.has_ipfs = true,
>> +	.mbox = {
>> +		.cmd = 0xe4,
>> +		.data_in = 0xe8,
>> +		.data_out = 0xec,
>> +		.owner = 0xf0,
>> +	},
>>  };
>>  MODULE_FIRMWARE("nvidia/tegra124/xusb.bin");
>>  
>> @@ -1397,6 +1411,12 @@ static const struct tegra_xusb_soc tegra210_soc = {
>>  	},
>>  	.scale_ss_clock = false,
>>  	.has_ipfs = true,
>> +	.mbox = {
>> +		.cmd = 0xe4,
>> +		.data_in = 0xe8,
>> +		.data_out = 0xec,
>> +		.owner = 0xf0,
>> +	},
>>  };
>>  MODULE_FIRMWARE("nvidia/tegra210/xusb.bin");
>>  
>> @@ -1422,6 +1442,12 @@ static const struct tegra_xusb_soc tegra186_soc = {
>>  	},
>>  	.scale_ss_clock = false,
>>  	.has_ipfs = false,
>> +	.mbox = {
>> +		.cmd = 0xe4,
>> +		.data_in = 0xe8,
>> +		.data_out = 0xec,
>> +		.owner = 0xf0,
>> +	},
> 
> This did not change any existing functionality, is there a follow-on
> patch somewhere that takes advantage of this change to enable different
> hardware?  Otherwise this doesn't seem worth it.
> 
Yes, I will submit another patch to enable Tegra194 XHCI. It will make use of
this patch to declare Tegra194 XUSB mailbox registers as:

	.mbox = {
		.cmd = 0x68,
		.data_in = 0x6c,
		.data_out = 0x70,
		.owner = 0x74,
	},

Thanks for review.
JC

> thanks,
> 
> greg k-h
>

Greg Kroah-Hartman Sept. 4, 2019, 5:21 a.m. UTC | #3

On Wed, Sep 04, 2019 at 09:43:08AM +0800, JC Kuo wrote:
> On 9/3/19 9:58 PM, Greg KH wrote:
> > On Mon, Sep 02, 2019 at 04:21:27PM +0800, JC Kuo wrote:
> >> Tegra194 XUSB host controller has rearranged mailbox registers. This
> >> commit makes mailbox registers address a part of "soc" data so that
> >> xhci-tegra driver can be used for Tegra194.
> >>
> >> Signed-off-by: JC Kuo <jckuo@nvidia.com>
> >> ---
> >>  drivers/usb/host/xhci-tegra.c | 58 +++++++++++++++++++++++++----------
> >>  1 file changed, 42 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> >> index dafc65911fc0..247b08ca49ee 100644
> >> --- a/drivers/usb/host/xhci-tegra.c
> >> +++ b/drivers/usb/host/xhci-tegra.c
> >> @@ -42,19 +42,18 @@
> >>  #define XUSB_CFG_CSB_BASE_ADDR			0x800
> >>  
> >>  /* FPCI mailbox registers */
> >> -#define XUSB_CFG_ARU_MBOX_CMD			0x0e4
> >> +/* XUSB_CFG_ARU_MBOX_CMD */
> >>  #define  MBOX_DEST_FALC				BIT(27)
> >>  #define  MBOX_DEST_PME				BIT(28)
> >>  #define  MBOX_DEST_SMI				BIT(29)
> >>  #define  MBOX_DEST_XHCI				BIT(30)
> >>  #define  MBOX_INT_EN				BIT(31)
> >> -#define XUSB_CFG_ARU_MBOX_DATA_IN		0x0e8
> >> +/* XUSB_CFG_ARU_MBOX_DATA_IN and XUSB_CFG_ARU_MBOX_DATA_OUT */
> >>  #define  CMD_DATA_SHIFT				0
> >>  #define  CMD_DATA_MASK				0xffffff
> >>  #define  CMD_TYPE_SHIFT				24
> >>  #define  CMD_TYPE_MASK				0xff
> >> -#define XUSB_CFG_ARU_MBOX_DATA_OUT		0x0ec
> >> -#define XUSB_CFG_ARU_MBOX_OWNER			0x0f0
> >> +/* XUSB_CFG_ARU_MBOX_OWNER */
> >>  #define  MBOX_OWNER_NONE			0
> >>  #define  MBOX_OWNER_FW				1
> >>  #define  MBOX_OWNER_SW				2
> >> @@ -146,6 +145,13 @@ struct tegra_xusb_phy_type {
> >>  	unsigned int num;
> >>  };
> >>  
> >> +struct tega_xusb_mbox_regs {
> >> +	unsigned int cmd;
> >> +	unsigned int data_in;
> >> +	unsigned int data_out;
> >> +	unsigned int owner;
> > 
> > Shouldn't these all be u8 values?
> > 
> These data members represent register offset in Tegra XUSB FPCI area. Size of
> FPCI area is 0x1000, so it is possible for future Tegra XUSB to have mailbox
> registers allocated to somewhere > 0x100.

Ok, then u16?

> > This did not change any existing functionality, is there a follow-on
> > patch somewhere that takes advantage of this change to enable different
> > hardware?  Otherwise this doesn't seem worth it.
> > 
> Yes, I will submit another patch to enable Tegra194 XHCI. It will make use of
> this patch to declare Tegra194 XUSB mailbox registers as:
> 
> 	.mbox = {
> 		.cmd = 0x68,
> 		.data_in = 0x6c,
> 		.data_out = 0x70,
> 		.owner = 0x74,
> 	},

Can you send that out as patch 2/2 so that we see the need for this
change?

thanks,

greg k-h

JC Kuo Sept. 4, 2019, 5:43 a.m. UTC | #4

On 9/4/19 1:21 PM, Greg KH wrote:
> On Wed, Sep 04, 2019 at 09:43:08AM +0800, JC Kuo wrote:
>> On 9/3/19 9:58 PM, Greg KH wrote:
>>> On Mon, Sep 02, 2019 at 04:21:27PM +0800, JC Kuo wrote:
>>>> Tegra194 XUSB host controller has rearranged mailbox registers. This
>>>> commit makes mailbox registers address a part of "soc" data so that
>>>> xhci-tegra driver can be used for Tegra194.
>>>>
>>>> Signed-off-by: JC Kuo <jckuo@nvidia.com>
>>>> ---
>>>>  drivers/usb/host/xhci-tegra.c | 58 +++++++++++++++++++++++++----------
>>>>  1 file changed, 42 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
>>>> index dafc65911fc0..247b08ca49ee 100644
>>>> --- a/drivers/usb/host/xhci-tegra.c
>>>> +++ b/drivers/usb/host/xhci-tegra.c
>>>> @@ -42,19 +42,18 @@
>>>>  #define XUSB_CFG_CSB_BASE_ADDR			0x800
>>>>  
>>>>  /* FPCI mailbox registers */
>>>> -#define XUSB_CFG_ARU_MBOX_CMD			0x0e4
>>>> +/* XUSB_CFG_ARU_MBOX_CMD */
>>>>  #define  MBOX_DEST_FALC				BIT(27)
>>>>  #define  MBOX_DEST_PME				BIT(28)
>>>>  #define  MBOX_DEST_SMI				BIT(29)
>>>>  #define  MBOX_DEST_XHCI				BIT(30)
>>>>  #define  MBOX_INT_EN				BIT(31)
>>>> -#define XUSB_CFG_ARU_MBOX_DATA_IN		0x0e8
>>>> +/* XUSB_CFG_ARU_MBOX_DATA_IN and XUSB_CFG_ARU_MBOX_DATA_OUT */
>>>>  #define  CMD_DATA_SHIFT				0
>>>>  #define  CMD_DATA_MASK				0xffffff
>>>>  #define  CMD_TYPE_SHIFT				24
>>>>  #define  CMD_TYPE_MASK				0xff
>>>> -#define XUSB_CFG_ARU_MBOX_DATA_OUT		0x0ec
>>>> -#define XUSB_CFG_ARU_MBOX_OWNER			0x0f0
>>>> +/* XUSB_CFG_ARU_MBOX_OWNER */
>>>>  #define  MBOX_OWNER_NONE			0
>>>>  #define  MBOX_OWNER_FW				1
>>>>  #define  MBOX_OWNER_SW				2
>>>> @@ -146,6 +145,13 @@ struct tegra_xusb_phy_type {
>>>>  	unsigned int num;
>>>>  };
>>>>  
>>>> +struct tega_xusb_mbox_regs {
>>>> +	unsigned int cmd;
>>>> +	unsigned int data_in;
>>>> +	unsigned int data_out;
>>>> +	unsigned int owner;
>>>
>>> Shouldn't these all be u8 values?
>>>
>> These data members represent register offset in Tegra XUSB FPCI area. Size of
>> FPCI area is 0x1000, so it is possible for future Tegra XUSB to have mailbox
>> registers allocated to somewhere > 0x100.
> 
> Ok, then u16?
> 

Thierry said he'd prefer to use "unsized" types to distinguish between register
offsets and values. I am fine with either way.

>>> This did not change any existing functionality, is there a follow-on
>>> patch somewhere that takes advantage of this change to enable different
>>> hardware?  Otherwise this doesn't seem worth it.
>>>
>> Yes, I will submit another patch to enable Tegra194 XHCI. It will make use of
>> this patch to declare Tegra194 XUSB mailbox registers as:
>>
>> 	.mbox = {
>> 		.cmd = 0x68,
>> 		.data_in = 0x6c,
>> 		.data_out = 0x70,
>> 		.owner = 0x74,
>> 	},
> 
> Can you send that out as patch 2/2 so that we see the need for this
> change?
> 
Yes, I will submit it along with this one.

Thanks,
JC

diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index dafc65911fc0..247b08ca49ee 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -42,19 +42,18 @@ 
 #define XUSB_CFG_CSB_BASE_ADDR			0x800
 
 /* FPCI mailbox registers */
-#define XUSB_CFG_ARU_MBOX_CMD			0x0e4
+/* XUSB_CFG_ARU_MBOX_CMD */
 #define  MBOX_DEST_FALC				BIT(27)
 #define  MBOX_DEST_PME				BIT(28)
 #define  MBOX_DEST_SMI				BIT(29)
 #define  MBOX_DEST_XHCI				BIT(30)
 #define  MBOX_INT_EN				BIT(31)
-#define XUSB_CFG_ARU_MBOX_DATA_IN		0x0e8
+/* XUSB_CFG_ARU_MBOX_DATA_IN and XUSB_CFG_ARU_MBOX_DATA_OUT */
 #define  CMD_DATA_SHIFT				0
 #define  CMD_DATA_MASK				0xffffff
 #define  CMD_TYPE_SHIFT				24
 #define  CMD_TYPE_MASK				0xff
-#define XUSB_CFG_ARU_MBOX_DATA_OUT		0x0ec
-#define XUSB_CFG_ARU_MBOX_OWNER			0x0f0
+/* XUSB_CFG_ARU_MBOX_OWNER */
 #define  MBOX_OWNER_NONE			0
 #define  MBOX_OWNER_FW				1
 #define  MBOX_OWNER_SW				2
@@ -146,6 +145,13 @@  struct tegra_xusb_phy_type {
 	unsigned int num;
 };
 
+struct tega_xusb_mbox_regs {
+	unsigned int cmd;
+	unsigned int data_in;
+	unsigned int data_out;
+	unsigned int owner;
+};
+
 struct tegra_xusb_soc {
 	const char *firmware;
 	const char * const *supply_names;
@@ -160,6 +166,8 @@  struct tegra_xusb_soc {
 		} usb2, ulpi, hsic, usb3;
 	} ports;
 
+	struct tega_xusb_mbox_regs mbox;
+
 	bool scale_ss_clock;
 	bool has_ipfs;
 };
@@ -395,15 +403,15 @@  static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
 	 * ACK/NAK messages.
 	 */
 	if (!(msg->cmd == MBOX_CMD_ACK || msg->cmd == MBOX_CMD_NAK)) {
-		value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_OWNER);
+		value = fpci_readl(tegra, tegra->soc->mbox.owner);
 		if (value != MBOX_OWNER_NONE) {
 			dev_err(tegra->dev, "mailbox is busy\n");
 			return -EBUSY;
 		}
 
-		fpci_writel(tegra, MBOX_OWNER_SW, XUSB_CFG_ARU_MBOX_OWNER);
+		fpci_writel(tegra, MBOX_OWNER_SW, tegra->soc->mbox.owner);
 
-		value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_OWNER);
+		value = fpci_readl(tegra, tegra->soc->mbox.owner);
 		if (value != MBOX_OWNER_SW) {
 			dev_err(tegra->dev, "failed to acquire mailbox\n");
 			return -EBUSY;
@@ -413,17 +421,17 @@  static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
 	}
 
 	value = tegra_xusb_mbox_pack(msg);
-	fpci_writel(tegra, value, XUSB_CFG_ARU_MBOX_DATA_IN);
+	fpci_writel(tegra, value, tegra->soc->mbox.data_in);
 
-	value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_CMD);
+	value = fpci_readl(tegra, tegra->soc->mbox.cmd);
 	value |= MBOX_INT_EN | MBOX_DEST_FALC;
-	fpci_writel(tegra, value, XUSB_CFG_ARU_MBOX_CMD);
+	fpci_writel(tegra, value, tegra->soc->mbox.cmd);
 
 	if (wait_for_idle) {
 		unsigned long timeout = jiffies + msecs_to_jiffies(250);
 
 		while (time_before(jiffies, timeout)) {
-			value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_OWNER);
+			value = fpci_readl(tegra, tegra->soc->mbox.owner);
 			if (value == MBOX_OWNER_NONE)
 				break;
 
@@ -431,7 +439,7 @@  static int tegra_xusb_mbox_send(struct tegra_xusb *tegra,
 		}
 
 		if (time_after(jiffies, timeout))
-			value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_OWNER);
+			value = fpci_readl(tegra, tegra->soc->mbox.owner);
 
 		if (value != MBOX_OWNER_NONE)
 			return -ETIMEDOUT;
@@ -598,16 +606,16 @@  static irqreturn_t tegra_xusb_mbox_thread(int irq, void *data)
 
 	mutex_lock(&tegra->lock);
 
-	value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_DATA_OUT);
+	value = fpci_readl(tegra, tegra->soc->mbox.data_out);
 	tegra_xusb_mbox_unpack(&msg, value);
 
-	value = fpci_readl(tegra, XUSB_CFG_ARU_MBOX_CMD);
+	value = fpci_readl(tegra, tegra->soc->mbox.cmd);
 	value &= ~MBOX_DEST_SMI;
-	fpci_writel(tegra, value, XUSB_CFG_ARU_MBOX_CMD);
+	fpci_writel(tegra, value, tegra->soc->mbox.cmd);
 
 	/* clear mailbox owner if no ACK/NAK is required */
 	if (!tegra_xusb_mbox_cmd_requires_ack(msg.cmd))
-		fpci_writel(tegra, MBOX_OWNER_NONE, XUSB_CFG_ARU_MBOX_OWNER);
+		fpci_writel(tegra, MBOX_OWNER_NONE, tegra->soc->mbox.owner);
 
 	tegra_xusb_mbox_handle(tegra, &msg);
 
@@ -1365,6 +1373,12 @@  static const struct tegra_xusb_soc tegra124_soc = {
 	},
 	.scale_ss_clock = true,
 	.has_ipfs = true,
+	.mbox = {
+		.cmd = 0xe4,
+		.data_in = 0xe8,
+		.data_out = 0xec,
+		.owner = 0xf0,
+	},
 };
 MODULE_FIRMWARE("nvidia/tegra124/xusb.bin");
 
@@ -1397,6 +1411,12 @@  static const struct tegra_xusb_soc tegra210_soc = {
 	},
 	.scale_ss_clock = false,
 	.has_ipfs = true,
+	.mbox = {
+		.cmd = 0xe4,
+		.data_in = 0xe8,
+		.data_out = 0xec,
+		.owner = 0xf0,
+	},
 };
 MODULE_FIRMWARE("nvidia/tegra210/xusb.bin");
 
@@ -1422,6 +1442,12 @@  static const struct tegra_xusb_soc tegra186_soc = {
 	},
 	.scale_ss_clock = false,
 	.has_ipfs = false,
+	.mbox = {
+		.cmd = 0xe4,
+		.data_in = 0xe8,
+		.data_out = 0xec,
+		.owner = 0xf0,
+	},
 };
 
 static const struct of_device_id tegra_xusb_of_match[] = {

xhci: tegra: Parameterize mailbox register addresses

Commit Message

Comments

Patch