[U-Boot] [PATCH v2 03/18] mmc: Add support for Qualcomm SDHCI controller

[Mateusz Kulikowski]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,

Ouch - I missed this one - sorry for late reply.

On 09.02.2016 22:33, Jagan Teki wrote:
> On 8 February 2016 at 02:27, Mateusz Kulikowski
> <mateusz.kulikowski at gmail.com> wrote:
[...]
>> +/* Non-standard registers needed for SDHCI startup */
>> +#define SDCC_MCI_POWER   0x0
>> +#define SDCC_MCI_POWER_SW_RST BIT(7)
>> +
>> +/* This is undocumented register */
>> +#define SDCC_MCI_VERSION             0x50
>> +#define SDCC_MCI_VERSION_MAJOR_SHIFT 28
>> +#define SDCC_MCI_VERSION_MAJOR_MASK  (0xf << SDCC_MCI_VERSION_MAJOR_SHIFT)
>> +#define SDCC_MCI_VERSION_MINOR_MASK  0xff
>> +
>> +#define SDCC_MCI_STATUS2 0x6C
>> +#define SDCC_MCI_STATUS2_MCI_ACT 0x1
>> +#define SDCC_MCI_HC_MODE 0x78
>> +
>> +/* Offset to SDHCI registers */
>> +#define SDCC_SDHCI_OFFSET 0x900
>> +
>> +/* Non standard (?) SDHCI register */
>> +#define SDHCI_VENDOR_SPEC_CAPABILITIES0  0x11c
> 
> IMHO, since this is based on Linux please use similar macro's to make
> easy changes in future.

I disagree (and I don't like names used on Linux on this particular driver);

"My" names are taken from Qualcomm documentation (HRD to be specific) 
except for VERSION that was named like that for coherency.

IMHO it's less confusing and allows faster documentation lookups.

Unless Linux names are from some (public) IP Core documentation - then 
please point me to it and I will do the rename (and add reference 
to it in the driver).

> 
[...]
>> +
>> +static const struct udevice_id msm_mmc_ids[] = {
>> +       { .compatible = "qcom,sdhci-msm-v4" },
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(msm_sdc_drv) = {
> 
> Same as above - msm_sdhci_drv looks more readable and with below
> driver attributes as well.

Ok, Agree - this will be more coherent with DT bindings / file names.

Regards,
Mateusz
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJWzjJGAAoJELvtohmVtQzBAlQIAIpXDp744j4xWUQYpq/wNhIO
MOmq3HlX+PBgTNPBKm9ub6XCTKa1K5DtGQ/VNKNDx/bv3fYyW7yTAoO4PP1/9eB5
NyM69NYfiQL8euw2y/LGHPqO3jL+T5FG+Y/PSx2GxtFUusGrlMXQG+f+IyhEvY0A
7F5hBK+OixzKlUvm8ZgP4dLzc3dLKI/BaZu7Hz7nzj3BaA2d0MpHUqYISfZU4TB+
mO+iHiFgWsu0yC+KhIxqZmxoVsCWvbVOmEdj37h5IAsxWgRRqQF8UhxTTGZpT7nk
U3G3oNPOwTZtsDtfqP2ia3phB2T6ZP6tys8AhNkRuAHEh9d5J7hylkxYwm0eE48=
=SXjw
-----END PGP SIGNATURE-----