| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <56BC428B.5030903@imgtec.com>
Date: Thu, 11 Feb 2016 08:12:59 +0000
From: Matt Redfearn <matt.redfearn@...tec.com>
To: David Daney <ddaney@...iumnetworks.com>
CC: <linux-mmc@...r.kernel.org>, <david.daney@...ium.com>,
<ulf.hansson@...aro.org>, <linux-mips@...ux-mips.org>,
<linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
<Zubair.Kakakhel@...tec.com>,
Aleksey Makarov <aleksey.makarov@...iumnetworks.com>,
Chandrakala Chavva <cchavva@...iumnetworks.com>,
Aleksey Makarov <aleksey.makarov@...iga.com>,
Leonid Rosenboim <lrosenboim@...iumnetworks.com>,
Peter Swain <pswain@...ium.com>,
Aaron Williams <aaron.williams@...ium.com>
Subject: Re: [PATCH v5] mmc: OCTEON: Add host driver for OCTEON MMC controller
Hi David
On 10/02/16 18:02, David Daney wrote:
> On 02/10/2016 09:36 AM, Matt Redfearn wrote:
>> From: Aleksey Makarov<aleksey.makarov@...iumnetworks.com>
>>
>> The OCTEON MMC controller is currently found on cn61XX and cnf71XX
>> devices. Device parameters are configured from device tree data.
>>
>> eMMC, MMC and SD devices are supported.
>>
>> Tested-by: Aaro Koskinen<aaro.koskinen@....fi>
>> Signed-off-by: Chandrakala Chavva<cchavva@...iumnetworks.com>
>> Signed-off-by: David Daney<david.daney@...ium.com>
>> Signed-off-by: Aleksey Makarov<aleksey.makarov@...iga.com>
>> Signed-off-by: Leonid Rosenboim<lrosenboim@...iumnetworks.com>
>> Signed-off-by: Peter Swain<pswain@...ium.com>
>> Signed-off-by: Aaron Williams<aaron.williams@...ium.com>
>> Signed-off-by: Matt Redfearn<matt.redfearn@...tec.com>
>> ---
>> v5:
>> Incoroprate comments from review
>> http://patchwork.linux-mips.org/patch/9558/
>> - Use standard <bus-width> property instead of <cavium,bus-max-width>.
>> - Use standard <max-frequency> property instead of <spi-max-frequency>.
>> - Add octeon_mmc_of_parse_legacy function to deal with the above
>> properties, since many devices have shipped with those properties
>> embedded in firmware.
>> - Allow the <vmmc-supply> binding in addition to the legacy
>> <gpios-power>.
>> - Remove the secondary driver for each slot.
>> - Use core gpio cd/wp handling
>>
> [...]
>
>> +static int octeon_mmc_of_copy_legacy_u32(struct device_node *node,
>> + const char *legacy_name,
>> + const char *new_name)
>> +{
>> + u32 value;
>> + int ret;
>> +
>> + ret = of_property_read_u32(node, legacy_name, &value);
>> + if (!ret) {
>> + /* Found legacy - set generic property */
>> + struct property *new_p;
>> + u32 *new_v;
>> +
>> + pr_warn(FW_WARN "%s: Legacy property '%s'. Please remove\n",
>> + node->full_name, legacy_name);
>> +
>
> I don't like this warning message.
>
> The vast majority of people that see it will not be able to change
> their firmware. So it will be forever cluttering up their boot logs.
>
> We are not ever planning on removing support for legacy firmware
> properties, so alarming people is really all this message does.
>
> If you insist on a message then make it something like pr_info("This
> is working properly, but please consider using modern device tree
> properties...")
Fair enough - I was just following what the PHY driver does when it
encounters a whitelisted compatible string, e.g. when my board boots:
[Firmware Warn]: /soc@...dio@...0000001800/ethernet-phy@0: Whitelisted
compatible string. Please remove
[Firmware Warn]: /soc@...dio@...0000001800/ethernet-phy@1: Whitelisted
compatible string. Please remove
[Firmware Warn]: /soc@...dio@...0000001800/ethernet-phy@2: Whitelisted
compatible string. Please remove
[Firmware Warn]: /soc@...dio@...0000001800/ethernet-phy@3: Whitelisted
compatible string. Please remove
[Firmware Warn]: /soc@...dio@...0000001800/ethernet-phy@4: Whitelisted
compatible string. Please remove
[Firmware Warn]: /soc@...dio@...0000001800/ethernet-phy@5: Whitelisted
compatible string. Please remove
[Firmware Warn]: /soc@...dio@...0000001800/ethernet-phy@6: Whitelisted
compatible string. Please remove
[Firmware Warn]: /soc@...dio@...0000001800/ethernet-phy@7: Whitelisted
compatible string. Please remove
But I can see why it would be preferable to avoid this kind of message.
I don't think it's essential so could be removed.
Thanks,
Matt
>
>> + new_p = kzalloc(sizeof(*new_p), GFP_KERNEL);
>> + new_v = kzalloc(sizeof(u32), GFP_KERNEL);
>> + if (!new_p || !new_v)
>> + return -ENOMEM;
>> +
>> + *new_v = value;
>> + new_p->name = kstrdup(new_name, GFP_KERNEL);
>> + new_p->length = sizeof(u32);
>> + new_p->value = new_v;
>> +
>> + of_update_property(node, new_p);
>> + }
>> + return 0;
>> +}
>> +
>> +/*
>> + * This function parses the legacy device tree that may be found in
>> devices
>> + * shipped before the driver was upstreamed. Future devices should
>> not require
>> + * it as standard bindings should be used
>> + */
>> +static int octeon_mmc_of_parse_legacy(struct device *dev,
>> + struct device_node *node,
>> + struct octeon_mmc_slot *slot)
>> +{
>> + int ret;
>> +
>> + ret = octeon_mmc_of_copy_legacy_u32(node, "cavium,bus-max-width",
>> + "bus-width");
>> + if (ret)
>> + return ret;
>> +
>> + ret = octeon_mmc_of_copy_legacy_u32(node, "spi-max-frequency",
>> + "max-frequency");
>> + if (ret)
>> + return ret;
>> +
>> + slot->pwr_gpiod = devm_gpiod_get_optional(dev, "power",
>> GPIOD_OUT_LOW);
>> + if (!IS_ERR(slot->pwr_gpiod)) {
>> + pr_warn(FW_WARN "%s: Legacy property '%s'. Please remove\n",
>> + node->full_name, "gpios-power");
>> + }
>> +
>> + return 0;
>> +}
>> +
>
Powered by blists - more mailing lists