lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0CE8B6BE3C4AD74AB97D9D29BD24E55202367762@CORPEXCH1.na.ads.idt.com>
Date:	Wed, 19 Oct 2011 12:54:06 -0700
From:	"Bounine, Alexandre" <Alexandre.Bounine@....com>
To:	Kumar Gala <galak@...nel.crashing.org>, <linuxppc-dev@...abs.org>
CC:	Andrew Morton <akpm@...l.org>, Liu Gang <Gang.Liu@...escale.com>,
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 3/5][v2] fsl-rio: Add two ports and rapidio message units support

On Thu, Oct 13, 2011 at 10:09 AM, Kumar Gala wrote:
> 
> From: Liu Gang <Gang.Liu@...escale.com>
> 
> Usually, freescale rapidio endpoint can support one 1X or two 4X LP-
> Serial
> link interfaces, and rapidio message transactions can be implemented
by
> two

Is the number of 1x ports described correctly?
Can we have two 1x ports as well? 

> message units. This patch adds the support of two rapidio ports and
> initializes message unit 0 and message unit 1. And these ports and
> message
... skip ...
> +
> +		/* Probe the master port phy type */
> +		ccsr = in_be32(priv->regs_win + RIO_CCSR + i*0x20);
> +		port->phy_type = (ccsr & 1) ? RIO_PHY_SERIAL :
> RIO_PHY_PARALLEL;
> +		dev_info(&dev->dev, "RapidIO PHY type: %s\n",
> +				(port->phy_type == RIO_PHY_PARALLEL) ?
> +				"parallel" :
> +				((port->phy_type == RIO_PHY_SERIAL) ?
"serial"
> :
> +				 "unknown"));
> +		/* Checking the port training status */
> +		if (in_be32((priv->regs_win + RIO_ESCSR + i*0x20)) & 1)
{
> +			dev_err(&dev->dev, "Port %d is not ready. "
> +				"Try to restart connection...\n", i);
> +			switch (port->phy_type) {
> +			case RIO_PHY_SERIAL:
> +				/* Disable ports */
> +				out_be32(priv->regs_win
> +					+ RIO_CCSR + i*0x20, 0);
> +				/* Set 1x lane */
> +				setbits32(priv->regs_win
> +					+ RIO_CCSR + i*0x20,
0x02000000);
> +				/* Enable ports */
> +				setbits32(priv->regs_win
> +					+ RIO_CCSR + i*0x20,
0x00600000);
> +				break;
> +			case RIO_PHY_PARALLEL:
> +				/* Disable ports */
> +				out_be32(priv->regs_win
> +					+ RIO_CCSR + i*0x20,
0x22000000);
> +				/* Enable ports */
> +				out_be32(priv->regs_win
> +					+ RIO_CCSR + i*0x20,
0x44000000);
> +				break;
> +			}

Probably this may be a good moment to drop the support for parallel
link.
Especially after you renamed controller to "srio" in the device tree.

> +			msleep(100);
> +			if (in_be32((priv->regs_win
> +					+ RIO_ESCSR + i*0x20)) & 1) {
> +				dev_err(&dev->dev,
> +					"Port %d restart failed.\n", i);
> +				release_resource(&port->iores);
> +				kfree(priv);
> +				kfree(port);
> +				continue;
> +			}
> +			dev_info(&dev->dev, "Port %d restart
success!\n", i);
> +		}
> +		fsl_rio_info(&dev->dev, ccsr);
> +
... skip ...
> 
>  struct rio_msg_regs {
> -	u32 omr;	/* 0xD_3000 - Outbound message 0 mode register
*/
> -	u32 osr;	/* 0xD_3004 - Outbound message 0 status register
*/
> +	u32 omr;
> +	u32 osr;
>  	u32 pad1;
> -	u32 odqdpar;	/* 0xD_300C - Outbound message 0 descriptor
> queue
> -			   dequeue pointer address register */
> +	u32 odqdpar;
>  	u32 pad2;
> -	u32 osar;	/* 0xD_3014 - Outbound message 0 source address
> -			   register */
> -	u32 odpr;	/* 0xD_3018 - Outbound message 0 destination
port
> -			   register */
> -	u32 odatr;	/* 0xD_301C - Outbound message 0 destination
> attributes
> -			   Register*/
> -	u32 odcr;	/* 0xD_3020 - Outbound message 0 double-word
count
> -			   register */
> +	u32 osar;
> +	u32 odpr;
> +	u32 odatr;
> +	u32 odcr;
>  	u32 pad3;
> -	u32 odqepar;	/* 0xD_3028 - Outbound message 0 descriptor
> queue
> -			   enqueue pointer address register */
> +	u32 odqepar;
>  	u32 pad4[13];
> -	u32 imr;	/* 0xD_3060 - Inbound message 0 mode register */
> -	u32 isr;	/* 0xD_3064 - Inbound message 0 status register
*/
> +	u32 imr;
> +	u32 isr;
>  	u32 pad5;
> -	u32 ifqdpar;	/* 0xD_306C - Inbound message 0 frame queue
> dequeue
> -			   pointer address register*/
> +	u32 ifqdpar;
>  	u32 pad6;
> -	u32 ifqepar;	/* 0xD_3074 - Inbound message 0 frame queue
> enqueue
> -			   pointer address register */
> -	u32 pad7[226];
> -	u32 odmr;	/* 0xD_3400 - Outbound doorbell mode register */
> -	u32 odsr;	/* 0xD_3404 - Outbound doorbell status register
*/
> +	u32 ifqepar;
> +	u32 pad7;

Do we need pad7 here?

> +};
> +
> +struct rio_dbell_regs {
> +	u32 odmr;
> +	u32 odsr;
>  	u32 res0[4];
> -	u32 oddpr;	/* 0xD_3418 - Outbound doorbell destination port
> -			   register */

... skip ...

> 
> @@ -340,35 +327,45 @@ fsl_rio_dbell_handler(int irq, void
> *dev_instance)
>  			" sid %2.2x tid %2.2x info %4.4x\n",
>  			DBELL_SID(dmsg), DBELL_TID(dmsg),
DBELL_INF(dmsg));
> 
> -		list_for_each_entry(dbell, &port->dbells, node) {
> -			if ((dbell->res->start <= DBELL_INF(dmsg)) &&
> -				(dbell->res->end >= DBELL_INF(dmsg))) {
> -				found = 1;
> -				break;
> +		for (i = 0; i < MAX_PORT_NUM; i++) {
> +			if (fsl_dbell->mport[i]) {
> +				list_for_each_entry(dbell,
> +					&fsl_dbell->mport[i]->dbells,
node) {
> +					if ((dbell->res->start
> +						<= DBELL_INF(dmsg))
> +						&& (dbell->res->end
> +						>= DBELL_INF(dmsg))) {
> +						found = 1;
> +						break;
> +					}
> +				}
> +				if (found && dbell->dinb) {
> +					dbell->dinb(fsl_dbell->mport[i],
> +						dbell->dev_id,
DBELL_SID(dmsg),
> +						DBELL_TID(dmsg),
> +						DBELL_INF(dmsg));
> +					break;
> +				}
>  			}
>  		}

Do we need to check for matching DBELL_TID and mport destID here
and scan only doorbell list attached to the right port? Otherwise this
may call service routine associated with doorbell on a wrong port.

> -		if (found) {
> -			dbell->dinb(port, dbell->dev_id,
> -					DBELL_SID(dmsg),
> -					DBELL_TID(dmsg),
DBELL_INF(dmsg));
> -		} else {
> +
> +		if (!found) {
>  			pr_debug
>  				("RIO: spurious doorbell,"
>  				" sid %2.2x tid %2.2x info %4.4x\n",
>  				DBELL_SID(dmsg), DBELL_TID(dmsg),
>  				DBELL_INF(dmsg));
>  		}
> -		setbits32(&rmu->msg_regs->dmr, DOORBELL_DMR_DI);
> -		out_be32(&rmu->msg_regs->dsr, DOORBELL_DSR_DIQI);
> +		setbits32(&fsl_dbell->dbell_regs->dmr, DOORBELL_DMR_DI);
> +		out_be32(&fsl_dbell->dbell_regs->dsr,
DOORBELL_DSR_DIQI);
>  	}
> 
>  out:
>  	return IRQ_HANDLED;
>  }
> 
... skip ...

> @@ -1114,50 +1104,48 @@ int fsl_rio_setup_rmu(struct rio_mport *mport,
> struct device_node *node)
>  {
>  	struct rio_priv *priv;
>  	struct fsl_rmu *rmu;
> -	struct rio_ops *ops;
> +	u64 msg_start;
> +	const u32 *msg_addr;
> +	int mlen;
> +	int aw;
> 
> -	if (!mport || !mport->priv || !node)
> -		return -1;
> +	if (!mport || !mport->priv)
> +		return -EFAULT;
EINVAL may be better here?

> +
> +	priv = mport->priv;
> +
> +	if (!node) {
> +		dev_warn(priv->dev, "Can't get %s property 'fsl,rmu'\n",
> +			priv->dev->of_node->full_name);
> +		return -EFAULT;
EINVAL as well?

> +	}

Regards,

Alex.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ