On 8/10/19 12:34 AM, Rob Herring wrote:
> On Fri, Aug 9, 2019 at 11:33 AM <marek.vasut@xxxxxxxxx> wrote:
>>
>> From: Marek Vasut <marek.vasut+renesas@xxxxxxxxx>
>>
>> The of_empty_ranges_quirk() returns a mix of boolean and signed integer
>> types, which cannot work well.
>
> It never returns a negative. The negative is used as an uninitialized
> flag. Note quirk_state is static.
It's still mixing boolean and signed int types though, which isn't right.
>> Replace that with boolean only and fix
>> usage logic in of_translate_one() -- the check should trigger when the
>> ranges are NULL and the quirk is applicable on the hardware.
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@xxxxxxxxx>
>> Cc: Rob Herring <robh+dt@xxxxxxxxxx>
>> Cc: Frank Rowand <frowand.list@xxxxxxxxx>
>> Cc: linux-renesas-soc@xxxxxxxxxxxxxxx
>> To: devicetree@xxxxxxxxxxxxxxx
>> ---
>> drivers/of/address.c | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/of/address.c b/drivers/of/address.c
>> index b492176c0572..ae2819e148b8 100644
>> --- a/drivers/of/address.c
>> +++ b/drivers/of/address.c
>> @@ -616,7 +616,7 @@ static struct of_bus *of_match_bus(struct device_node *np)
>> return NULL;
>> }
>>
>> -static int of_empty_ranges_quirk(struct device_node *np)
>> +static bool of_empty_ranges_quirk(struct device_node *np)
>> {
>> if (IS_ENABLED(CONFIG_PPC)) {
>> /* To save cycles, we cache the result for global "Mac" setting */
>> @@ -631,7 +631,8 @@ static int of_empty_ranges_quirk(struct device_node *np)
>> quirk_state =
>> of_machine_is_compatible("Power Macintosh") ||
>> of_machine_is_compatible("MacRISC");
>> - return quirk_state;
>> + if (quirk_state > 0)
>> + return true;
>> }
>> return false;
>> }
>> @@ -662,8 +663,8 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
>> * This code is only enabled on powerpc. --gcl
>> */
>> ranges = of_get_property(parent, rprop, &rlen);
>> - if (ranges == NULL && !of_empty_ranges_quirk(parent)) {
>> - pr_debug("no ranges; cannot translate\n");
>> + if (ranges == NULL && of_empty_ranges_quirk(parent)) {
>> + pr_err("no ranges; cannot translate\n");
>
> This is wrong. If you have NULL ranges and not the quirk, then no
> ranges is an error. IOW, if you are getting an error here, you have an
> error in your DT (because I assume you are not working on a PASemi or
> Apple system).
The way I understand the code is that
if (you have no ranges in the DT) AND (the quirk is applicable) then
print the message. Which is what this patch does.
Am I missing something ?
--
Best regards,
Marek Vasut
