Re: [PATCHv4 2/3] ARM: socfpga: dts: Add support for SD/MMC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/21/2013 01:48 PM, Dinh Nguyen wrote:
> On Fri, 2013-08-16 at 16:36 -0600, Stephen Warren wrote:
>> On 08/14/2013 10:48 AM, dinguyen@xxxxxxxxxx wrote:
>>> From: Dinh Nguyen <dinguyen@xxxxxxxxxx>
>>>
>>> Add bindings for SD/MMC for SOCFPGA.
>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/socfpga-dw-mshc.txt
>>
>>> +* altr,sysmgr: Should be the phandle to the system_mgr node. As this is where
>>> +		this where the register that controls the CIU clock phases
>>> +		reside.
>>
>> On the surface, this binding series seems OK, but I do have a question:
>> how is the sysmgr phandle used?
>>
>> I assume there's some register in this syscon device that resets or
>> enables or otherwise controls this MSHC module. How does the code know
>> which register it is? The phandle in the altr,sysmgr property would
>> usually be followed by a/some cell(s) that encode this information, so
>> that the MSHC driver doesn't have to know anything about the layout of
>> the syscon registers, and so the sysconf driver doesn't have to know
>> anything about the identity of the MSHC client device.
> 
> There is a #define SYSMGR_SDMMCGRP_CTRL_OFFSET that is in
> dw_mmc-socfpga.c. This defines the offset from the base address that the
> sysmgr phandle will give me.

Hmmm. That doesn't sound good. That means that the SDMMC driver knows
internal details about some other HW module. It'd be better if either:

a) The sysmgr driver was required to provide an API to the SDMMC driver
to set up the CIU register as requested.

or:

b) The CIU register details were represented in DT.

Either of these would allow the SDMMC driver to operate unchanged on an
SoC with a different sysmgr register layout.
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux