linux-kernel - Re: [PATCH 1/2] misc: ezport-firmware: new 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] [day] [month] [year] [list]

Date:   Fri, 23 Mar 2018 15:06:32 +0100
From:   Sebastian Reichel <sebastian.reichel@...labora.co.uk>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Arnd Bergmann <arnd@...db.de>, Shawn Guo <shawnguo@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Sascha Hauer <kernel@...gutronix.de>,
        Fabio Estevam <fabio.estevam@....com>,
        Mark Rutland <mark.rutland@....com>,
        Nandor Han <nandor.han@...com>, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, kernel@...labora.com
Subject: Re: [PATCH 1/2] misc: ezport-firmware: new driver

Hi,

On Fri, Mar 23, 2018 at 02:17:12PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Mar 20, 2018 at 06:22:00PM +0100, Sebastian Reichel wrote:
> > General Electric Healthcare's PPD has a secondary processor from
> > NXP's Kinetis K20 series. It's firmware can be updated from Linux
> > using the EzPort interface. This driver implements the firmware
> > updating process.
> > 
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@...labora.co.uk>
> > ---
> >  Documentation/devicetree/bindings/misc/ge-achc.txt |  19 +-
> 
> Bindings should be in a separate patch, right?

Right.

> > +static int ezport_read_data(struct spi_device *spi, u32 address,
> > +			    u8 *buffer, size_t size)
> > +{
> > +	struct spi_transfer xfers[2] = {};
> 
> You can send SPI data from the stack?

I don't think so:

/**
 * struct spi_transfer - a read/write buffer pair
 * @tx_buf: data to be written (dma-safe memory), or NULL
 * @rx_buf: data to be read (dma-safe memory), or NULL

IIUIC only spi_write_then_read() may use the stack, since the
function copies the provided buffer to a second, DMA capable
buffer. That function is obviously not good for big buffers,
though.

> > +	u8 *query = kmalloc(EZPORT_FAST_READ_SIZE, GFP_KERNEL);
> > +	int ret;
> > +
> > +	if (!query)
> > +		return -ENOMEM;
> > +
> > +	query[0] = EZPORT_CMD_FAST_READ;
> > +	query[1] = address >> 16;
> > +	query[2] = address >> 8;
> > +	query[3] = address >> 0;
> > +	query[4] = EZPORT_DUMMY;
> > +
> > +	xfers[0].len = EZPORT_FAST_READ_SIZE;
> > +	xfers[0].tx_buf = query;
> > +
> > +	xfers[1].len = size;
> > +	xfers[1].rx_buf = buffer;
> > +
> > +	ret = spi_sync_transfer(spi, xfers, 2);
> > +
> > +	kfree(query);
> > +
> > +	return ret;
> > +}
> > +static DEVICE_ATTR_WO(update_fw);
> > +static DEVICE_ATTR_RO(verify_fw);
> 
> New sysfs attributes need Documentation/ABI updates please.

Ok.

> Also please fix up the kbuild-reported build errors.

Of course.

> thanks,

Thanks for the review,

-- Sebastian

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)