linux-kernel - Re: [PATCH 2/3] spi: of: allow instantiating slaves without a driver

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: <CAOMqctSA0G+fBKNgAizqWvEUfJLr9Fh0SsN0Pn-cQv=zeu7iFg@mail.gmail.com>
Date:	Sun, 26 Jun 2016 04:23:41 +0200
From:	Michal Suchanek <hramrach@...il.com>
To:	Mark Brown <broonie@...nel.org>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Ingo Molnar <mingo@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Kees Cook <keescook@...omium.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Dan Williams <dan.j.williams@...el.com>,
	Tejun Heo <tj@...nel.org>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Davidlohr Bueso <dave@...olabs.net>,
	Andrey Ryabinin <aryabinin@...tuozzo.com>,
	Nikolay Aleksandrov <nikolay@...ulusnetworks.com>,
	Dmitry Vyukov <dvyukov@...gle.com>,
	Adrien Schildknecht <adrien+dev@...ischi.me>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-spi <linux-spi@...r.kernel.org>
Subject: Re: [PATCH 2/3] spi: of: allow instantiating slaves without a driver

On 26 June 2016 at 03:15, Mark Brown <broonie@...nel.org> wrote:
> On Thu, Jun 23, 2016 at 05:41:20PM -0000, Michal Suchanek wrote:
>> SPI slave devices are not created when looking up driver for the slave
>> fails. Create a device anyway so it can be manually bound to a driver.
>
>> @@ -1543,11 +1542,10 @@ of_register_spi_device(struct spi_master *master, struct device_node *nc)
>>       /* Device speed */
>>       rc = of_property_read_u32(nc, "spi-max-frequency", &value);
>>       if (rc) {
>> -             dev_err(&master->dev, "%s has no valid 'spi-max-frequency' property (%d)\n",
>> +             dev_warn(&master->dev, "%s has no valid 'spi-max-frequency' property (%d)\n",
>>                       nc->full_name, rc);
>> -             goto err_out;
>> -     }
>> -     spi->max_speed_hz = value;
>> +     } else
>> +             spi->max_speed_hz = value;
>>
>
> I can't relate this hunk to the changelog and there's a coding style
> problem, if there's { } on one side of an if statement it should be on
> both sides.  Why are we making this change?

The intention is that you can specify that your SPI master controller
has a CS available without setting up the slave

&spi2 {
        pinctrl-names = "default";
        pinctrl-0 = <&spi2_pins_a>,
                    <&spi2_cs0_pins_a>;
        status = "okay";

        uext_spi: spi@...t {
                reg = <0>;
        };
};

Then you can amend the slave node with an overlay or bind a driver
that can deal with the node having no configuration.

The check here isn't very effective anyway since slaves with zero
speed somehow creep into the kernel. I have seen people reporting
division by zero in SPI master driver as a result. The proper way to
fix this is to specify the master minimum and maximum speed limit so
the SPI core can reject transfers with speed outside of the allowed
range.

Thanks

Michal