From: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> To: Marek Vasut <marex-ynQEQJNshbs@public.gmane.org> Cc: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>, Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>, Attila Kinali <attila-HB9FjVmMKa7tRgLqZ5aouw@public.gmane.org>, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, Chris Ball <cjb-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org>, Dong Aisheng <b29396-KZfg59tc24xl57MIdRCFDg@public.gmane.org>, Linux ARM kernel <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org> Subject: Re: [PATCH 06/10 V2] spi: Add SPI driver for mx233/mx28 Date: Wed, 1 Aug 2012 21:31:07 +0100 [thread overview] Message-ID: <20120801203106.GW4483@opensource.wolfsonmicro.com> (raw) In-Reply-To: <1343076052-27312-7-git-send-email-marex-ynQEQJNshbs@public.gmane.org> On Mon, Jul 23, 2012 at 10:40:48PM +0200, Marek Vasut wrote: > This is slightly reworked version of the SPI driver. > Support for DT has been added and it's been converted > to queued API. Looks reasonable overall. > + bits_per_word = dev->bits_per_word; > + if (t && t->bits_per_word) > + bits_per_word = t->bits_per_word; > + > + if (bits_per_word != 8) { > + dev_err(&dev->dev, "%s, unsupported bits_per_word=%d\n", > + __func__, bits_per_word); > + return -EINVAL; > + } > + if (dev->max_speed_hz) > + hz = dev->max_speed_hz; > + if (t && t->speed_hz) > + hz = t->speed_hz; > + if (hz == 0) { > + dev_err(&dev->dev, "Cannot continue with zero clock\n"); > + return -EINVAL; > + } These two blocks use a different style (the first does the first assign unconditionally, the second uses initialisation with declaration). I prefer the first style but YMMV and it doesn't matter. For the missing clock rate might it make sense to just use whatever the clock happens to come out as? > +static void mxs_spi_cleanup(struct spi_device *dev) > +{ > + return; > +} Empty functions are generally a warning sign... why do we need this one? > +static int __devexit mxs_spi_remove(struct platform_device *pdev) > +{ > + clk_disable_unprepare(ssp->clk); It'd be nice to only keep the clocks enabled while doing transfers but again totally non-essential. ------------------------------------------------------------------------------ Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
WARNING: multiple messages have this Message-ID (diff)
From: broonie@opensource.wolfsonmicro.com (Mark Brown) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 06/10 V2] spi: Add SPI driver for mx233/mx28 Date: Wed, 1 Aug 2012 21:31:07 +0100 [thread overview] Message-ID: <20120801203106.GW4483@opensource.wolfsonmicro.com> (raw) In-Reply-To: <1343076052-27312-7-git-send-email-marex@denx.de> On Mon, Jul 23, 2012 at 10:40:48PM +0200, Marek Vasut wrote: > This is slightly reworked version of the SPI driver. > Support for DT has been added and it's been converted > to queued API. Looks reasonable overall. > + bits_per_word = dev->bits_per_word; > + if (t && t->bits_per_word) > + bits_per_word = t->bits_per_word; > + > + if (bits_per_word != 8) { > + dev_err(&dev->dev, "%s, unsupported bits_per_word=%d\n", > + __func__, bits_per_word); > + return -EINVAL; > + } > + if (dev->max_speed_hz) > + hz = dev->max_speed_hz; > + if (t && t->speed_hz) > + hz = t->speed_hz; > + if (hz == 0) { > + dev_err(&dev->dev, "Cannot continue with zero clock\n"); > + return -EINVAL; > + } These two blocks use a different style (the first does the first assign unconditionally, the second uses initialisation with declaration). I prefer the first style but YMMV and it doesn't matter. For the missing clock rate might it make sense to just use whatever the clock happens to come out as? > +static void mxs_spi_cleanup(struct spi_device *dev) > +{ > + return; > +} Empty functions are generally a warning sign... why do we need this one? > +static int __devexit mxs_spi_remove(struct platform_device *pdev) > +{ > + clk_disable_unprepare(ssp->clk); It'd be nice to only keep the clocks enabled while doing transfers but again totally non-essential.
next prev parent reply other threads:[~2012-08-01 20:31 UTC|newest] Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top 2012-07-23 20:40 [PATCH 00/10 V2] MXS SPI driver Marek Vasut 2012-07-23 20:40 ` Marek Vasut [not found] ` <1343076052-27312-1-git-send-email-marex-ynQEQJNshbs@public.gmane.org> 2012-07-23 20:40 ` [PATCH 01/10 RESEND] mmc: spi: Move SSP register definitions into separate file Marek Vasut 2012-07-23 20:40 ` Marek Vasut 2012-07-23 20:40 ` [PATCH 02/10 RESEND] mmc: spi: Rename IMX2[38]_MMC to IMX2[38]_SSP Marek Vasut 2012-07-23 20:40 ` Marek Vasut 2012-07-23 20:40 ` [PATCH 03/10 V2] mmc: spi: Add necessary bits into mxs-spi.h Marek Vasut 2012-07-23 20:40 ` Marek Vasut 2012-07-23 20:40 ` [PATCH 04/10 RESEND] mmc: spi: Pull out parts shared between MMC and SPI Marek Vasut 2012-07-23 20:40 ` Marek Vasut 2012-07-23 20:40 ` [PATCH 05/10 V2] mmc: spi: Pull out the SSP clock configuration function Marek Vasut 2012-07-23 20:40 ` Marek Vasut 2012-07-23 20:40 ` [PATCH 06/10 V2] spi: Add SPI driver for mx233/mx28 Marek Vasut 2012-07-23 20:40 ` Marek Vasut [not found] ` <1343076052-27312-7-git-send-email-marex-ynQEQJNshbs@public.gmane.org> 2012-08-01 20:31 ` Mark Brown [this message] 2012-08-01 20:31 ` Mark Brown [not found] ` <20120801203106.GW4483-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> 2012-08-02 14:58 ` Marek Vasut 2012-08-02 14:58 ` Marek Vasut [not found] ` <201208021658.38237.marex-ynQEQJNshbs@public.gmane.org> 2012-08-02 16:00 ` Mark Brown 2012-08-02 16:00 ` Mark Brown [not found] ` <20120802160016.GA4537-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> 2012-08-02 16:03 ` Marek Vasut 2012-08-02 16:03 ` Marek Vasut 2012-08-03 1:29 ` Shawn Guo 2012-08-03 1:29 ` Shawn Guo 2012-08-03 13:38 ` Thomas Petazzoni 2012-08-03 13:38 ` Thomas Petazzoni 2012-08-03 13:46 ` Fabio Estevam 2012-08-03 13:46 ` Fabio Estevam [not found] ` <CAOMZO5AQs1NzJBogcLPcKtY=QSaxpDmzUTvpfScY7P10FEoMJw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2012-08-03 13:49 ` Marek Vasut 2012-08-03 13:49 ` Marek Vasut 2012-08-03 14:00 ` Marek Vasut 2012-08-03 14:00 ` Marek Vasut 2012-07-23 20:40 ` [PATCH 07/10 RESEND] mmc: spi: Pull out common DMA parts from MXS MMC Marek Vasut 2012-07-23 20:40 ` Marek Vasut 2012-07-23 20:40 ` [PATCH 08/10 RESEND] spi: Add DMA support into SPI driver Marek Vasut 2012-07-23 20:40 ` Marek Vasut [not found] ` <1343076052-27312-9-git-send-email-marex-ynQEQJNshbs@public.gmane.org> 2012-08-01 20:34 ` Mark Brown 2012-08-01 20:34 ` Mark Brown [not found] ` <20120801203448.GX4483-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> 2012-08-02 15:00 ` Marek Vasut 2012-08-02 15:00 ` Marek Vasut 2012-07-23 20:40 ` [PATCH 09/10 RESEND] spi: Add SSP/SPI device tree documentation Marek Vasut 2012-07-23 20:40 ` Marek Vasut 2012-07-24 18:15 ` Sergei Shtylyov 2012-07-24 18:15 ` Sergei Shtylyov [not found] ` <500EE64B.8080103-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org> 2012-07-24 19:43 ` Marek Vasut 2012-07-24 19:43 ` Marek Vasut [not found] ` <201207242143.49038.marex-ynQEQJNshbs@public.gmane.org> 2012-07-28 11:40 ` Shawn Guo 2012-07-28 11:40 ` Shawn Guo [not found] ` <20120728113957.GE2128-rvtDTF3kK1ictlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org> 2012-07-28 11:42 ` Marek Vasut 2012-07-28 11:42 ` Marek Vasut 2012-07-23 20:40 ` [PATCH 10/10 RESEND] ARM: mx28: Add SPI pinmux into imx28.dtsi Marek Vasut 2012-07-23 20:40 ` Marek Vasut 2012-07-24 9:25 ` [PATCH 00/10 V2] MXS SPI driver Attila Kinali 2012-07-24 9:25 ` Attila Kinali 2012-08-03 14:30 ` [PATCH] SPI: MXS: Allow to pass the SPI master bus number from the device tree Maxime Ripard 2012-08-03 14:30 ` Maxime Ripard [not found] ` <1344004239-22868-1-git-send-email-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> 2012-08-03 14:43 ` Shawn Guo 2012-08-03 14:43 ` Shawn Guo [not found] ` <20120803144326.GE23791-rvtDTF3kK1ictlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org> 2012-08-03 16:27 ` Maxime Ripard 2012-08-03 16:27 ` Maxime Ripard -- strict thread matches above, loose matches on Subject: below -- 2012-07-06 6:17 [PATCH 01/10 RESEND] mmc: spi: Move SSP register definitions into separate file Marek Vasut [not found] ` <1341555449-17507-1-git-send-email-marex-ynQEQJNshbs@public.gmane.org> 2012-07-06 6:17 ` [PATCH 06/10 V2] spi: Add SPI driver for mx233/mx28 Marek Vasut 2012-07-06 6:17 ` Marek Vasut
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20120801203106.GW4483@opensource.wolfsonmicro.com \ --to=broonie-yzvpicuk2aatku/dhu1wvuem+bqzidxxqq4iyu8u01e@public.gmane.org \ --cc=attila-HB9FjVmMKa7tRgLqZ5aouw@public.gmane.org \ --cc=b29396-KZfg59tc24xl57MIdRCFDg@public.gmane.org \ --cc=cjb-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org \ --cc=fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org \ --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \ --cc=marex-ynQEQJNshbs@public.gmane.org \ --cc=shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \ --cc=spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.