From: Guennadi Liakhovetski <g.liakhovetski@gmx.de> To: Simon Horman <horms@verge.net.au> Cc: Arnd Bergmann <arnd@arndb.de>, linux-mmc@vger.kernel.org, devicetree-discuss@lists.ozlabs.org, linux-sh@vger.kernel.org, Magnus Damm <magnus.damm@gmail.com> Subject: Re: [PATCH v3 06/13] mmc: tmio-mmc: define device-tree bindings Date: Wed, 13 Feb 2013 15:59:42 +0000 [thread overview] Message-ID: <Pine.LNX.4.64.1302131632280.18535@axis700.grange> (raw) In-Reply-To: <20130207005957.GB9775@verge.net.au> On Thu, 7 Feb 2013, Simon Horman wrote: > On Wed, Feb 06, 2013 at 10:24:20PM +0000, Arnd Bergmann wrote: > > On Wednesday 06 February 2013, Guennadi Liakhovetski wrote: > > > +* Toshiba Mobile IO SD/MMC controller > > > + > > > +The tmio-mmc driver doesn't probe its devices actively, instead its binding to > > > +devices is managed by either MFD drivers or by the sh_mobile_sdhi platform > > > +driver. Those drivers supply the tmio-mmc driver with platform data, that either > > > +describe hardware capabilities, known to them, or are obtained by them from > > > +their own platform data or from their DT information. In the latter case all > > > +compulsory and any optional properties, common to all SD/MMC drivers, as > > > +described in mmc.txt, should or can be used. Additionally the following optional > > > +bindings can be used. They set respective TMIO_MMC_* flags. > > > + > > > +Optional properties: > > > +- toshiba,mmc-wrprotect-disable : set TMIO_MMC_WRPROTECT_DISABLE flag > > > +- toshiba,mmc-blksz-2bytes : set TMIO_MMC_BLKSZ_2BYTES > > > +- toshiba,mmc-has-idle-wait : set TMIO_MMC_HAS_IDLE_WAIT > > > > Please write the binding in a way that does not refer to a specific > > implementation in Linux: The binding should describe the hardware > > independent of details in the driver. In particular, I think you > > should not refer to the TMIO_MMC_BLKSZ_2BYTES etc macros but describe > > in text what the flags are about. > > > > Regarding the toshiba,mmc-wrprotect-disable property, would it be > > enough to just check the presence of the wp-gpios property? > > > > TMIO_MMC_BLKSZ_2BYTES seems to be set unconditionally in > > sh_mobile_sdhi_probe and nowhere else, so I'd assume we don't > > actually need to provide this here, but can keep that knowledge > > implicit based on whether we're talking to sh_mobile_sdhi > > or another tmio_mmc variant. Can do that, yes. > > For the other last one, is that actually board specific, or just > > a feature of a given chip? If we can tell by the SoC, then I'd > > suggest using separate "compatible" properties instead, and > > put a bitmask of features into the .data field of the of match > > table. For all I can tell, SH7372 does not set it, while SH73A0, > > R8A7740 and R8A7779 always do. > > My understanding is that TMIO_MMC_HAS_IDLE_WAIT can be set based > on the SoC in use. So far TMIO_MMC_HAS_IDLE_WAIT is set on board-kzm9g.c (sh73a0 / AG5) board-ag5evm.c (sh73a0 / AG5) board-armadillo800eva.c (r8a7740 / A1) board-kota2.c (sh73a0 / AG5) board-marzen.c (r8a7779 / H1) and isn't set on board-ap4evb.c (sh7372 / ap4) board-bonito.c (r8a7740 / a1, SDHI isn't used) board-mackerel.c (sh7372 / ap4) So, shall we use a compatible property for this and drop this property? We can add later at any time, if needed, which is better, than defining something redundant. OTOH I seem to remember, that using SoC-version from the "compatible" property was considered by someone inappropriate. Magnus, what do you think? So, if we drop TMIO_MMC_BLKSZ_2BYTES and TMIO_MMC_HAS_IDLE_WAIT, we only keep toshiba,mmc-wrprotect-disable to set TMIO_MMC_WRPROTECT_DISABLE? And that one is definitely needed, because that even differs between SDHI instances on one SoC. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
WARNING: multiple messages have this Message-ID (diff)
From: Guennadi Liakhovetski <g.liakhovetski@gmx.de> To: Simon Horman <horms@verge.net.au> Cc: Arnd Bergmann <arnd@arndb.de>, linux-mmc@vger.kernel.org, devicetree-discuss@lists.ozlabs.org, linux-sh@vger.kernel.org, Magnus Damm <magnus.damm@gmail.com> Subject: Re: [PATCH v3 06/13] mmc: tmio-mmc: define device-tree bindings Date: Wed, 13 Feb 2013 16:59:42 +0100 (CET) [thread overview] Message-ID: <Pine.LNX.4.64.1302131632280.18535@axis700.grange> (raw) In-Reply-To: <20130207005957.GB9775@verge.net.au> On Thu, 7 Feb 2013, Simon Horman wrote: > On Wed, Feb 06, 2013 at 10:24:20PM +0000, Arnd Bergmann wrote: > > On Wednesday 06 February 2013, Guennadi Liakhovetski wrote: > > > +* Toshiba Mobile IO SD/MMC controller > > > + > > > +The tmio-mmc driver doesn't probe its devices actively, instead its binding to > > > +devices is managed by either MFD drivers or by the sh_mobile_sdhi platform > > > +driver. Those drivers supply the tmio-mmc driver with platform data, that either > > > +describe hardware capabilities, known to them, or are obtained by them from > > > +their own platform data or from their DT information. In the latter case all > > > +compulsory and any optional properties, common to all SD/MMC drivers, as > > > +described in mmc.txt, should or can be used. Additionally the following optional > > > +bindings can be used. They set respective TMIO_MMC_* flags. > > > + > > > +Optional properties: > > > +- toshiba,mmc-wrprotect-disable : set TMIO_MMC_WRPROTECT_DISABLE flag > > > +- toshiba,mmc-blksz-2bytes : set TMIO_MMC_BLKSZ_2BYTES > > > +- toshiba,mmc-has-idle-wait : set TMIO_MMC_HAS_IDLE_WAIT > > > > Please write the binding in a way that does not refer to a specific > > implementation in Linux: The binding should describe the hardware > > independent of details in the driver. In particular, I think you > > should not refer to the TMIO_MMC_BLKSZ_2BYTES etc macros but describe > > in text what the flags are about. > > > > Regarding the toshiba,mmc-wrprotect-disable property, would it be > > enough to just check the presence of the wp-gpios property? > > > > TMIO_MMC_BLKSZ_2BYTES seems to be set unconditionally in > > sh_mobile_sdhi_probe and nowhere else, so I'd assume we don't > > actually need to provide this here, but can keep that knowledge > > implicit based on whether we're talking to sh_mobile_sdhi > > or another tmio_mmc variant. Can do that, yes. > > For the other last one, is that actually board specific, or just > > a feature of a given chip? If we can tell by the SoC, then I'd > > suggest using separate "compatible" properties instead, and > > put a bitmask of features into the .data field of the of match > > table. For all I can tell, SH7372 does not set it, while SH73A0, > > R8A7740 and R8A7779 always do. > > My understanding is that TMIO_MMC_HAS_IDLE_WAIT can be set based > on the SoC in use. So far TMIO_MMC_HAS_IDLE_WAIT is set on board-kzm9g.c (sh73a0 / AG5) board-ag5evm.c (sh73a0 / AG5) board-armadillo800eva.c (r8a7740 / A1) board-kota2.c (sh73a0 / AG5) board-marzen.c (r8a7779 / H1) and isn't set on board-ap4evb.c (sh7372 / ap4) board-bonito.c (r8a7740 / a1, SDHI isn't used) board-mackerel.c (sh7372 / ap4) So, shall we use a compatible property for this and drop this property? We can add later at any time, if needed, which is better, than defining something redundant. OTOH I seem to remember, that using SoC-version from the "compatible" property was considered by someone inappropriate. Magnus, what do you think? So, if we drop TMIO_MMC_BLKSZ_2BYTES and TMIO_MMC_HAS_IDLE_WAIT, we only keep toshiba,mmc-wrprotect-disable to set TMIO_MMC_WRPROTECT_DISABLE? And that one is definitely needed, because that even differs between SDHI instances on one SoC. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/
next prev parent reply other threads:[~2013-02-13 15:59 UTC|newest] Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-02-06 19:46 [PATCH v3 00/13] mmc: core and driver DT and related development Guennadi Liakhovetski 2013-02-06 19:46 ` Guennadi Liakhovetski 2013-02-06 19:46 ` [PATCH v3 01/13] mmc: sdhi, tmio: only check flags in tmio-mmc driver proper Guennadi Liakhovetski 2013-02-06 19:46 ` Guennadi Liakhovetski 2013-02-06 19:46 ` [PATCH v3 02/13] mmc: detailed definition of CD and WP MMC line polarities in DT Guennadi Liakhovetski 2013-02-06 19:46 ` Guennadi Liakhovetski 2013-02-06 22:30 ` Arnd Bergmann 2013-02-06 22:30 ` Arnd Bergmann 2013-02-06 19:46 ` [PATCH v3 03/13] mmc: provide a standard MMC device-tree binding parser centrally Guennadi Liakhovetski 2013-02-06 19:46 ` Guennadi Liakhovetski 2013-02-09 13:37 ` Markus Pargmann 2013-02-09 13:37 ` Markus Pargmann 2013-02-06 19:46 ` [PATCH v3 04/13] mmc: (cosmetic) remove "extern" from function declarations Guennadi Liakhovetski 2013-02-06 19:46 ` Guennadi Liakhovetski 2013-02-06 19:46 ` [PATCH v3 05/13] mmc: sh-mmcif: use mmc_of_parse() to parse standard MMC DT bindings Guennadi Liakhovetski 2013-02-06 19:46 ` Guennadi Liakhovetski [not found] ` <1360180020-18555-1-git-send-email-g.liakhovetski-Mmb7MZpHnFY@public.gmane.org> 2013-02-06 19:46 ` [PATCH v3 06/13] mmc: tmio-mmc: define device-tree bindings Guennadi Liakhovetski 2013-02-06 19:46 ` Guennadi Liakhovetski [not found] ` <1360180020-18555-7-git-send-email-g.liakhovetski-Mmb7MZpHnFY@public.gmane.org> 2013-02-06 22:24 ` Arnd Bergmann 2013-02-06 22:24 ` Arnd Bergmann 2013-02-07 0:59 ` Simon Horman 2013-02-07 0:59 ` Simon Horman 2013-02-07 8:27 ` Arnd Bergmann 2013-02-07 8:27 ` Arnd Bergmann 2013-02-13 15:59 ` Guennadi Liakhovetski [this message] 2013-02-13 15:59 ` Guennadi Liakhovetski 2013-02-14 1:42 ` Magnus Damm 2013-02-14 1:42 ` Magnus Damm 2013-02-14 1:59 ` Simon Horman 2013-02-14 1:59 ` Simon Horman 2013-02-14 2:09 ` Magnus Damm 2013-02-14 2:09 ` Magnus Damm 2013-02-14 2:24 ` Simon Horman 2013-02-14 2:24 ` Simon Horman 2013-02-14 8:28 ` Guennadi Liakhovetski 2013-02-14 8:28 ` Guennadi Liakhovetski 2013-02-14 9:12 ` Magnus Damm 2013-02-14 9:12 ` Magnus Damm 2013-02-14 9:25 ` Guennadi Liakhovetski 2013-02-14 9:25 ` Guennadi Liakhovetski 2013-02-14 15:51 ` Arnd Bergmann 2013-02-06 19:46 ` [PATCH v3 07/13] mmc: tmio-mmc: parse " Guennadi Liakhovetski 2013-02-06 19:46 ` Guennadi Liakhovetski 2013-02-06 19:46 ` [PATCH v3 08/13] mmc: sh_mobile_sdhi: remove unused .pdata field Guennadi Liakhovetski 2013-02-06 19:46 ` Guennadi Liakhovetski 2013-02-06 19:46 ` [PATCH v3 09/13] mmc: sh_mobile_sdhi: use managed resource allocations Guennadi Liakhovetski 2013-02-06 19:46 ` Guennadi Liakhovetski 2013-02-06 19:46 ` [PATCH v3 10/13] mmc: tmio: remove unused and deprecated symbols Guennadi Liakhovetski 2013-02-06 19:46 ` Guennadi Liakhovetski 2013-02-06 19:46 ` [PATCH v3 11/13] mmc: tmio: add support for the VccQ regulator Guennadi Liakhovetski 2013-02-06 19:46 ` Guennadi Liakhovetski 2013-02-06 19:46 ` [PATCH v3 12/13] mmc: add DT bindings for more MMC capability flags Guennadi Liakhovetski 2013-02-06 19:46 ` Guennadi Liakhovetski 2013-02-06 19:47 ` [PATCH v3 13/13] mmc: tmio: add barriers to IO operations Guennadi Liakhovetski 2013-02-06 19:47 ` Guennadi Liakhovetski 2013-02-07 9:51 ` Paul Mundt 2013-02-07 9:51 ` Paul Mundt
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=Pine.LNX.4.64.1302131632280.18535@axis700.grange \ --to=g.liakhovetski@gmx.de \ --cc=arnd@arndb.de \ --cc=devicetree-discuss@lists.ozlabs.org \ --cc=horms@verge.net.au \ --cc=linux-mmc@vger.kernel.org \ --cc=linux-sh@vger.kernel.org \ --cc=magnus.damm@gmail.com \ /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.