linux-kernel - RE: Re: [PATCH] spi: imx: add a check for speed

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-next>] [day] [month] [year] [list]

Message-ID: <AM6PR04MB56233BE0FFF7E5D80527B54CF3419@AM6PR04MB5623.eurprd04.prod.outlook.com>
Date:   Tue, 27 Apr 2021 08:33:06 +0000
From:   Clark Wang <xiaoning.wang@....com>
To:     Mark Brown <broonie@...nel.org>
CC:     "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        dl-linux-imx <linux-imx@....com>,
        "linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: Re: [PATCH] spi: imx: add a check for speed_hz before calculating
 the clock


> -----Original Message-----
> From: Mark Brown <broonie@...nel.org>
> Sent: Thursday, April 8, 2021 21:44
> To: Clark Wang <xiaoning.wang@....com>
> Cc: shawnguo@...nel.org; s.hauer@...gutronix.de; festevam@...il.com;
> kernel@...gutronix.de; dl-linux-imx <linux-imx@....com>; linux-
> spi@...r.kernel.org; linux-arm-kernel@...ts.infradead.org; linux-
> kernel@...r.kernel.org
> Subject: [EXT] Re: [PATCH] spi: imx: add a check for speed_hz before
calculating
> the clock
> 
> On Thu, Apr 08, 2021 at 06:33:47PM +0800, Clark Wang wrote:
> > When some drivers use spi to send data, spi_transfer->speed_hz is not
> > assigned. If spidev->max_speed_hz is not assigned as well, it will
> > cause an error in configuring the clock.
> 
> > Add a check for these two values before configuring the clock. An
> > error will be returned when they are not assigned.
> 
> For the case where the transfer speed is not set __spi_validate() will
take the
> controller's maximum speed so the controller should just be able to
> unconditionally use the transfer's speed.  Your issue is therefore that
the
> controllers are sometimes not setting a maximum speed which this doesn't
seem
> to fix AFAICT?  I'd expect the driver to be able to work one out based on
the
> input clock.

Hi Mark,

Yes, you are right. If the t->speed_hz is not provided, it will use
spi->max_speed_hz.
This patch is not needed.
The issue I met is that t->speed_hz is not provided in slave mode. But this
is normal in slave mode.
The problem is spi-imx should not config the clock divider in slave mode. I
will send a new patch to disable the clock configuration in slave mode
later.

However, I notice that you have applied this patch to the next branch?
Will you revert this patch?
I think you may want to apply this patch I sent before.

Author: Clark Wang <xiaoning.wang@....com>
Date:   Mon Dec 14 17:05:04 2020 +0800

    spi: imx: add 16/32 bits per word support for slave mode
    
    Enable 16/32 bits per word support for spi-imx slave mode.
    It only support 8 bits per word in slave mode before.
    
    Signed-off-by: Clark Wang <xiaoning.wang@....com>
    Reviewed-by: Haibo Chen <haibo.chen@....com>

Thank you very much! :)

Best Regards,
Clark Wang

> 
> >  struct spi_imx_devtype_data {
> >  	void (*intctrl)(struct spi_imx_data *, int);
> >  	int (*prepare_message)(struct spi_imx_data *, struct spi_message *);
> > -	int (*prepare_transfer)(struct spi_imx_data *, struct spi_device *,
> > -				struct spi_transfer *);
> > +	int (*prepare_transfer)(struct spi_imx_data *, struct spi_device *);
> >  	void (*trigger)(struct spi_imx_data *);
> >  	int (*rx_available)(struct spi_imx_data *);
> >  	void (*reset)(struct spi_imx_data *);
> 
> This seems to be a fairly big and surprising refactoring for the described
change.
> It's quite hard to tie the change to the changelog.

Download attachment "smime.p7s" of type "application/pkcs7-signature" (9583 bytes)