linux-kernel - Re: [PATCH] spi: tegra: add spi driver for SLINK controller

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]

Date:	Mon, 29 Oct 2012 09:17:54 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	Laxman Dewangan <ldewangan@...dia.com>
CC:	"broonie@...nsource.wolfsonmicro.com" 
	<broonie@...nsource.wolfsonmicro.com>,
	"grant.likely@...retlab.ca" <grant.likely@...retlab.ca>,
	"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
	"spi-devel-general@...ts.sourceforge.net" 
	<spi-devel-general@...ts.sourceforge.net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH] spi: tegra: add spi driver for SLINK controller

On 10/26/2012 12:49 PM, Laxman Dewangan wrote:
> Thanks Stephen for review.
> I have taken care of almost all feedback. Some of having my below comments.
> 
> On Tuesday 23 October 2012 01:32 AM, Stephen Warren wrote:
>> On 10/18/2012 04:47 AM, Laxman Dewangan wrote:
>>> Tegra20/Tegra30 supports the spi interface through its SLINK
>>> controller. Add spi driver for SLINK controller.

>>> +     val_write = SLINK_RDY;
>>> +     val_write |= (val&  SLINK_FIFO_ERROR);
>> Why not just always set SLINK_FIFO_ERROR; does it have to be set in the
>> write only if the status was previously asserted? If that is true, how
>> do you avoid a race condition where the bit gets set in SLINK_STATUS
>> after you read it but before you write to clear it?
>
> Status gets updated together. There is no steps of updating status.

Sorry, I don't understand this answer.

>>> +     spin_lock_irqsave(&tspi->lock, flags);
>>> +     if (err) {
>>> +             dev_err(tspi->dev,
>>> +                     "DmaXfer: ERROR bit set 0x%x\n",
>>> tspi->status_reg);
>>> +             dev_err(tspi->dev,
>>> +                     "DmaXfer 0x%08x:0x%08x:0x%08x\n",
>>> tspi->command_reg,
>>> +                             tspi->command2_reg,
>>> tspi->dma_control_reg);
>>> +             tegra_periph_reset_assert(tspi->clk);
>>> +             udelay(2);
>>> +             tegra_periph_reset_deassert(tspi->clk);
>> Do we /have/ to reset the SPI block; can't we just disable it in the
>> control register, clear all status, and re-program it from scratch?
>>
>> If at all possible, I would like to avoid introducing any new use of
>> tegra_periph_reset_{,de}assert, since that API has no standard subsystem
>> equivalent (or if it does, isn't hooked into the standard subsystem
>> yet), and hence means this driver relies on a header file currently in
>> arch/arm/mach-tegra/include/mach, and we need to move or delete all such
>> headers in order to support single zImage.
> 
> Is there a way to support the reset of controller. We will need this
> functionality.

Why do we need to reset the controller at all; can't we simply program
all the (few) configuration registers? Are there HW bugs that hang the
controller and require a reset or something?
--
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/