Hi,
On 09/07/2015 11:56 AM, vikas wrote:
> Hi,
>
> On 09/06/2015 08:16 AM, Marek Vasut wrote:
>> On Saturday, September 05, 2015 at 01:45:01 AM, vikas wrote:
>>> Hi,
>>>
>>> On 08/21/2015 02:20 AM, Marek Vasut wrote:
>>>> From: Graham Moore <grmoore@xxxxxxxxxxxxxxxxxxxxx>
>>>>
>>>> Add support for the Cadence QSPI controller. This controller is
>>>> present in the Altera SoCFPGA SoCs and this driver has been tested
>>>> on the Cyclone V SoC.
>>>
>>> can we add info about the modes supported/not supported like direct mode,
>>> indirect etc.
>>
>> It's already part of the documentation.
>
> To be clear, add info for modes supported in the driver. e.g. Direct mode is not supported in the driver.
> Lets add this info to help users.
>
>>
>>>> Signed-off-by: Graham Moore <grmoore@xxxxxxxxxxxxxxxxxxxxx>
>>>> Signed-off-by: Marek Vasut <marex@xxxxxxx>
>>>> Cc: Alan Tull <atull@xxxxxxxxxxxxxxxxxxxxx>
>>>> Cc: Brian Norris <computersforpeace@xxxxxxxxx>
>>>> Cc: David Woodhouse <dwmw2@xxxxxxxxxxxxx>
>>>> Cc: Dinh Nguyen <dinguyen@xxxxxxxxxxxxxxxxxxxxx>
>>>> Cc: Graham Moore <grmoore@xxxxxxxxxxxxxxxxxxxxx>
>>>> Cc: Vikas MANOCHA <vikas.manocha@xxxxxx>
>>>> Cc: Yves Vandervennet <yvanderv@xxxxxxxxxxxxxxxxxxxxx>
>>>> Cc: devicetree@xxxxxxxxxxxxxxx
>>>> ---
>>
>> [...]
>>
>>>> +#define CQSPI_REG_CMDADDRESS 0x94
>>>> +#define CQSPI_REG_CMDREADDATALOWER 0xA0
>>>> +#define CQSPI_REG_CMDREADDATAUPPER 0xA4
>>>> +#define CQSPI_REG_CMDWRITEDATALOWER 0xA8
>>>> +#define CQSPI_REG_CMDWRITEDATAUPPER 0xAC
>>>> +
[...]
>>>> +
>>>> + /* Clear all interrupts. */
>>>> + writel(CQSPI_IRQ_STATUS_MASK, reg_base + CQSPI_REG_IRQSTATUS);
>>>> +
>>>> + writel(CQSPI_IRQ_MASK_RD, reg_base + CQSPI_REG_IRQMASK);
>>>
>>> I think there is no need for separate masks for read & write. Use one mask
>>> & configure it once in the init rather than configuring each time for
>>> every read/write. Then in the ISR, take action as per the interrupt
>>> source: read/write/error condition etc.
>>
>> Setting up the specific IRQ mask prevents spurious interrupts during the
>> particular IO operation, so this solution looks more precise to me.
>
> spurious interrupt ? like ?
>
> Configuring interrupt at one time for read/write (preferably in init) is better software design
> then breaking it in for every read/write.
just to clarify "preferably in init" not always but yes in this case.
Cheers,
Vikas
>
>>
>>>> +
>>>> + reinit_completion(&cqspi->transfer_complete);
>>>> + writel(CQSPI_REG_INDIRECTRD_START_MASK,
>>>> + reg_base + CQSPI_REG_INDIRECTRD);
>>>> +
>>>> + while (remaining > 0) {
>>>> + ret =
[...]
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
