Message ID | 0e60b4b5-5016-e1c3-d29d-b81a41f64f48@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Sun, Sep 04, 2016 at 09:58:02AM +0200, Heiner Kallweit wrote: > Currently the driver allocates a 64kb buffer with each single message. > On systems with little and fragmented memory this can result in > memory allocation errors. Solve this by pre-allocating a buffer. > > This patch was developed in OpenWRT long ago, however it never > made it upstream. Author: Gabor Juhos <juhosg@openwrt.org> > > I slightly modified the original patch to re-initialize the buffer > at the beginning of each transfer. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> When you sent this previously I asked what happened with the signoff from Gabor. A little Google tells me that he did in fact sign off the copy that's in OpenWRT. Please don't drop signoffs, they're important for tracking licensing - see SubmittingPatches for details. If the modifications you made really are slight as you say then I'd also expect Gabor's authorship to be preserved.
Am 06.09.2016 um 12:58 schrieb Mark Brown: > On Sun, Sep 04, 2016 at 09:58:02AM +0200, Heiner Kallweit wrote: >> Currently the driver allocates a 64kb buffer with each single message. >> On systems with little and fragmented memory this can result in >> memory allocation errors. Solve this by pre-allocating a buffer. >> >> This patch was developed in OpenWRT long ago, however it never >> made it upstream. Author: Gabor Juhos <juhosg@openwrt.org> >> >> I slightly modified the original patch to re-initialize the buffer >> at the beginning of each transfer. >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > When you sent this previously I asked what happened with the signoff > from Gabor. A little Google tells me that he did in fact sign off the > copy that's in OpenWRT. Please don't drop signoffs, they're important > for tracking licensing - see SubmittingPatches for details. If the > modifications you made really are slight as you say then I'd also expect > Gabor's authorship to be preserved. > Sorry, then I misunderstood your comment. I'll resend the patch with Gabor's Signed-off-by. Do you want my Signed-off-by in addition? -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Sep 06, 2016 at 10:57:49PM +0200, Heiner Kallweit wrote: > Sorry, then I misunderstood your comment. > I'll resend the patch with Gabor's Signed-off-by. Do you want my > Signed-off-by in addition? You need your signoff on anything you send including this - like I say see SubmittingPatches for what this is doing.
diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c index a9004fe..a3a75ae 100644 --- a/drivers/spi/spi-fsl-espi.c +++ b/drivers/spi/spi-fsl-espi.c @@ -300,57 +300,44 @@ static void fsl_espi_do_trans(struct spi_message *m, static void fsl_espi_cmd_trans(struct spi_message *m, struct fsl_espi_transfer *trans, u8 *rx_buff) { + struct mpc8xxx_spi *mspi = spi_master_get_devdata(m->spi->master); struct spi_transfer *t; - u8 *local_buf; int i = 0; struct fsl_espi_transfer *espi_trans = trans; - local_buf = kzalloc(SPCOM_TRANLEN_MAX, GFP_KERNEL); - if (!local_buf) { - espi_trans->status = -ENOMEM; - return; - } - list_for_each_entry(t, &m->transfers, transfer_list) { if (t->tx_buf) { - memcpy(local_buf + i, t->tx_buf, t->len); + memcpy(mspi->local_buf + i, t->tx_buf, t->len); i += t->len; } } - espi_trans->tx_buf = local_buf; - espi_trans->rx_buf = local_buf; + espi_trans->tx_buf = mspi->local_buf; + espi_trans->rx_buf = mspi->local_buf; fsl_espi_do_trans(m, espi_trans); espi_trans->actual_length = espi_trans->len; - kfree(local_buf); } static void fsl_espi_rw_trans(struct spi_message *m, struct fsl_espi_transfer *trans, u8 *rx_buff) { + struct mpc8xxx_spi *mspi = spi_master_get_devdata(m->spi->master); struct spi_transfer *t; - u8 *local_buf; unsigned int tx_only = 0; int i = 0; - local_buf = kzalloc(SPCOM_TRANLEN_MAX, GFP_KERNEL); - if (!local_buf) { - trans->status = -ENOMEM; - return; - } - list_for_each_entry(t, &m->transfers, transfer_list) { if (t->tx_buf) { - memcpy(local_buf + i, t->tx_buf, t->len); + memcpy(mspi->local_buf + i, t->tx_buf, t->len); i += t->len; if (!t->rx_buf) tx_only += t->len; } } - trans->tx_buf = local_buf; - trans->rx_buf = local_buf; + trans->tx_buf = mspi->local_buf; + trans->rx_buf = mspi->local_buf; fsl_espi_do_trans(m, trans); if (!trans->status) { @@ -360,18 +347,19 @@ static void fsl_espi_rw_trans(struct spi_message *m, trans->len - tx_only); trans->actual_length += trans->len; } - - kfree(local_buf); } static int fsl_espi_do_one_msg(struct spi_master *master, struct spi_message *m) { + struct mpc8xxx_spi *mspi = spi_master_get_devdata(master); struct spi_transfer *t; u8 *rx_buf = NULL; unsigned int xfer_len = 0; struct fsl_espi_transfer espi_trans; + memset(mspi->local_buf, 0, SPCOM_TRANLEN_MAX); + list_for_each_entry(t, &m->transfers, transfer_list) { if (t->rx_buf) rx_buf = t->rx_buf; @@ -617,6 +605,13 @@ static struct spi_master * fsl_espi_probe(struct device *dev, mpc8xxx_spi = spi_master_get_devdata(master); + mpc8xxx_spi->local_buf = + devm_kmalloc(dev, SPCOM_TRANLEN_MAX, GFP_KERNEL); + if (!mpc8xxx_spi->local_buf) { + ret = -ENOMEM; + goto err_probe; + } + mpc8xxx_spi->reg_base = devm_ioremap_resource(dev, mem); if (IS_ERR(mpc8xxx_spi->reg_base)) { ret = PTR_ERR(mpc8xxx_spi->reg_base); diff --git a/drivers/spi/spi-fsl-lib.h b/drivers/spi/spi-fsl-lib.h index 84f5dcb..065b9db 100644 --- a/drivers/spi/spi-fsl-lib.h +++ b/drivers/spi/spi-fsl-lib.h @@ -30,6 +30,7 @@ struct mpc8xxx_spi { void *rx; #if IS_ENABLED(CONFIG_SPI_FSL_ESPI) int len; + u8 *local_buf; #endif int subblock;
Currently the driver allocates a 64kb buffer with each single message. On systems with little and fragmented memory this can result in memory allocation errors. Solve this by pre-allocating a buffer. This patch was developed in OpenWRT long ago, however it never made it upstream. Author: Gabor Juhos <juhosg@openwrt.org> I slightly modified the original patch to re-initialize the buffer at the beginning of each transfer. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- v2: - fixed coding style issue - fixed commit message --- drivers/spi/spi-fsl-espi.c | 41 ++++++++++++++++++----------------------- drivers/spi/spi-fsl-lib.h | 1 + 2 files changed, 19 insertions(+), 23 deletions(-)