On 07/03/16 13:32, Alan Cooper wrote:
> On Fri, Mar 4, 2016 at 7:48 PM, Kevin Cernekee <cernekee@xxxxxxxxx> wrote:
>> On Fri, Mar 4, 2016 at 10:07 AM, Al Cooper <alcooperx@xxxxxxxxx> wrote:
>>> Signed-off-by: Al Cooper <alcooperx@xxxxxxxxx>
>>> ---
>>> .../devicetree/bindings/mmc/sdhci-brcmstb.txt | 40 ++++++
>>> MAINTAINERS | 7 +
>>> drivers/mmc/host/Kconfig | 11 ++
>>> drivers/mmc/host/Makefile | 1 +
>>> drivers/mmc/host/sdhci-brcmstb.c | 154 +++++++++++++++++++++
>>> 5 files changed, 213 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-brcmstb.txt
>>> create mode 100644 drivers/mmc/host/sdhci-brcmstb.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-brcmstb.txt b/Documentation/devicetree/bindings/mmc/sdhci-brcmstb.txt
>>> new file mode 100644
>>> index 0000000..f8dd6a9d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-brcmstb.txt
>>> @@ -0,0 +1,40 @@
>>> +* BROADCOM BRCMSTB/BMIPS SDHCI Controller
>>> +
>>> +This file documents differences between the core properties in mmc.txt
>>> +and the properties used by the sdhci-brcmstb driver.
>>> +
>>> +Required properties:
>>> +- compatible: "brcm,sdhci-brcmstb"
>>
>> For many of the other SoC drivers, such as drivers/irqchip/irq-bcm*,
>> we've used the compatible string and/or driver name to denote the
>> oldest SoC that is compatible with the driver. This helps to indicate
>> that the driver is not compatible with previous versions of the core.
>> For instance, the driver you are submitting would be slightly broken
>> on the BCM7468/BCM7208 STB chips, and horrifically broken on BCM7630
>> (which admittedly isn't quite "brcmstb" but uses some of the same
>> drivers). I assume you don't want to target those ancient parts.
>>
>> When the hardware block changes frequently, we've gone as far as
>> creating a different compatible string for each variant. For
>> Documentation/devicetree/bindings/net/brcm,bcmgenet.txt we encoded the
>> hardware generation number in the compatible string.
>
> Something like "brcm,bcm7425-sdhci"?
You certainly know the history more than I do, but that sounds like an
appropriate compatible string here. We could list all chips that have
such a controller and have been quirky, that is how we are supposed to
solve that problem, but that's up to you.
>
>>
>> [snip]
>>
>>> +static int sdhci_brcmstb_probe(struct platform_device *pdev)
>>> +{
>>> + struct device_node *dn = pdev->dev.of_node;
>>> + struct sdhci_host *host;
>>> + struct sdhci_pltfm_host *pltfm_host;
>>> + struct clk *clk;
>>> + int res;
>>> +
>>> + clk = of_clk_get_by_name(dn, "sw_sdio");
>>> + if (IS_ERR(clk)) {
>>> + dev_err(&pdev->dev, "Clock not found in Device Tree\n");
>>> + clk = NULL;
>>
>> Last time I worked on these chips, they defaulted to "all clocks
>> enabled" in the absence of a clk driver.
>>
>> So, if this is not treated as a fatal condition, maybe it would be
>> better to downgrade it to dev_warn or dev_info.
>
> Good point, I'll switch it to dev_warn.
Since you are going to revisit this part, the binding should also
document the need for an optional clocks property, since it currently
does not.
--
Florian
--
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
