Hi, Pekon
2013/8/27 Gupta, Pekon <pekon@xxxxxx>:
>>
>> fix two things in patch:
>> commit id:f477b7fb13df2b843997559ff34e87d054ba6538
>>
>> 1.change the name of property: spi-tx-nbits and spi-rx-nbits to
>> spi-tmax-nbits and spi-rmax-nbits, aimed to make that name different
>> from the member of spi_transfer(tx_nbits and rx_nbits). using tmax
>> and rmax here to describe that it is the max transfer bits that the
>> members in spi_transfer(tx_nbits and rx_nbits) can reach.
>> 2.change property spi-tmax-nbits and spi-rmax-nbits to optional.
>> If User don't set spi-tmax-nbits or spi-rmax-nbits, spi device mode
>> should be regarded as SINGLE, not return an error. In such case, user
>> don't have to modify the old dts files to fit the new spi framework.
>>
>> Signed-off-by: wangyuhang <wangyuhang2014@xxxxxxxxx>
>> ---
>> drivers/spi/spi.c | 72 +++++++++++++++++++++++++--------------------------
>> --
>> 1 file changed, 34 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
>> index 50f7fc3..8d191f2 100644
>> --- a/drivers/spi/spi.c
>> +++ b/drivers/spi/spi.c
>> @@ -870,48 +870,44 @@ static void of_register_spi_devices(struct
>> spi_master *master)
>> spi->mode |= SPI_3WIRE;
>>
>> /* Device DUAL/QUAD mode */
>> - prop = of_get_property(nc, "spi-tx-nbits", &len);
>> + prop = of_get_property(nc, "spi-tmax-nbits", &len);
>> if (!prop || len < sizeof(*prop)) {
>> - dev_err(&master->dev, "%s has no 'spi-tx-nbits'
>> property\n",
>> - nc->full_name);
>> - spi_dev_put(spi);
>> - continue;
>> - }
>> - switch (be32_to_cpup(prop)) {
>> - case SPI_NBITS_SINGLE:
>> - break;
>> - case SPI_NBITS_DUAL:
>> - spi->mode |= SPI_TX_DUAL;
>> - break;
>> - case SPI_NBITS_QUAD:
>> - spi->mode |= SPI_TX_QUAD;
>> - break;
>> - default:
>> - dev_err(&master->dev, "spi-tx-nbits value is not
>> supported\n");
>> - spi_dev_put(spi);
>> - continue;
>> + /* set tx mode in SINGLE as default */
>> + } else {
>> + switch (be32_to_cpup(prop)) {
>> + case SPI_NBITS_SINGLE:
>> + break;
>> + case SPI_NBITS_DUAL:
>> + spi->mode |= SPI_TX_DUAL;
>> + break;
>> + case SPI_NBITS_QUAD:
>> + spi->mode |= SPI_TX_QUAD;
>> + break;
>> + default:
>> + dev_err(&master->dev,
>> + "spi-tmax-nbits value is not
>> supported\n");
>> + spi_dev_put(spi);
>> + continue;
>> }
>>
>> - prop = of_get_property(nc, "spi-rx-nbits", &len);
>> + prop = of_get_property(nc, "spi-rmax-nbits", &len);
>> if (!prop || len < sizeof(*prop)) {
>> - dev_err(&master->dev, "%s has no 'spi-rx-nbits'
>> property\n",
>> - nc->full_name);
>> - spi_dev_put(spi);
>> - continue;
>> - }
>> - switch (be32_to_cpup(prop)) {
>> - case SPI_NBITS_SINGLE:
>> - break;
>> - case SPI_NBITS_DUAL:
>> - spi->mode |= SPI_RX_DUAL;
>> - break;
>> - case SPI_NBITS_QUAD:
>> - spi->mode |= SPI_RX_QUAD;
>> - break;
>> - default:
>> - dev_err(&master->dev, "spi-rx-nbits value is not
>> supported\n");
>> - spi_dev_put(spi);
>> - continue;
>> + /* set rx mode in SINGLE as default */
>> + } else {
>> + switch (be32_to_cpup(prop)) {
>> + case SPI_NBITS_SINGLE:
>> + break;
>> + case SPI_NBITS_DUAL:
>> + spi->mode |= SPI_RX_DUAL;
>> + break;
>> + case SPI_NBITS_QUAD:
>> + spi->mode |= SPI_RX_QUAD;
>> + break;
>> + default:
>> + dev_err(&master->dev,
>> + "spi-rmax-nbits value is not
>> supported\n");
>> + spi_dev_put(spi);
>> + continue;
>> }
>>
>> /* Device speed */
>> --
>> 1.7.9.5
>
> You missed out comments provided earlier..
> http://lists.infradead.org/pipermail/linux-mtd/2013-August/048374.html
>
> Remember .. this compatibility check is between spi_device->mode
> v/s spi_master->mode_bits. This is different from ur V3 patch
> which checks for spi_transfer->tx_nbits v/s spi_device->mode.
>
> + if ((xfer->tx_nbits == SPI_NBITS_DUAL) &&
> + !(spi->mode & (SPI_TX_DUAL | SPI_TX_QUAD)))
> + return -EINVAL;
> + if ((xfer->tx_nbits == SPI_NBITS_QUAD) &&
> + !(spi->mode & SPI_TX_QUAD))
> + return -EINVAL;
>
>
Sorry for my mis-understanding. But here I want to regard DUAL / QUAD
mode just as other mode, such as CPHA, CPOL and so on. To other mode,
spi framework didn't do such check, example:
---------------------------------------------------------------------------------
/* Mode (clock phase/polarity/etc.) */
if (of_find_property(nc, "spi-cpha", NULL))
spi->mode |= SPI_CPHA;
if (of_find_property(nc, "spi-cpol", NULL))
spi->mode |= SPI_CPOL;
---------------------------------------------------------------------------------
Spi framework do this check in spi_setup as follow:
---------------------------------------------------------------------------------
bad_bits = spi->mode & ~spi->master->mode_bits;
if (bad_bits) {
dev_err(&spi->dev, "setup: unsupported mode bits %x\n",
bad_bits);
return -EINVAL;
}
---------------------------------------------------------------------------------
So to keep that consistant, I don't think it is necessary to add the
check when setting the mode.
Best regards
--
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
