All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
To: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Masahiro Yamada
	<yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>,
	"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Frank Rowand
	<frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Grant Likely
	<grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Ley Foon Tan <lftan-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org>
Subject: Re: [PATCH] of/fdt: fix error checking for earlycon address
Date: Wed, 28 Oct 2015 10:56:22 -0400	[thread overview]
Message-ID: <5630E216.908@hurleysoftware.com> (raw)
In-Reply-To: <CAL_JsqK6UJxvGt3_y5T+VMxpr7G=PCmQ_Nx0QW-EfeTHOCDMKQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 10/27/2015 04:46 PM, Rob Herring wrote:
> On Fri, Oct 23, 2015 at 8:15 AM, Peter Hurley <peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org> wrote:
>> Hi Masahiro,
>>
>> On 10/23/2015 07:47 AM, Masahiro Yamada wrote:
>>> fdt_translate_address() returns OF_BAD_ADDR on error.  It is defined as
>>> a u64 value, so the variable "addr" should be defined as u64 as well.
>>
>> Good catch.
>>
>> I would prefer if fdt_translate_address() returned resource_size_t (which
>> is the proper type for handling addresses as integers) and that type
>> was propagated through early_init_dt_scan_chosen_serial => of_setup_earlycon.
> 
> That can be problematic in the DT code. The size of resource_size_t
> can vary on 32-bit as it is based on phys_addr_t which in turn is
> based on CONFIG_LPAE setting. However, the address sizes in DT are
> fixed and may be 64-bit. So we stick with u64 in the DT code.

I think I see your point about fdt_translate_address(); you want it to
mirror of_translate_address(), yes?  If so, then I have no objection
to this patch and will mark it reviewed.

But the u64 value from fdt_translate_address() should be preserved
at least to the ioremap()/set_fixmap() in earlycon_map() .
I could roll that change into my earlycon series that moves
fdt_translate_address() into of_setup_earlycon(), thus preserving
the u64 value until it's assigned to a phys_addr_t.

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Peter Hurley <peter@hurleysoftware.com>
To: Rob Herring <robh@kernel.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-serial@vger.kernel.org" <linux-serial@vger.kernel.org>,
	Frank Rowand <frowand.list@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Grant Likely <grant.likely@linaro.org>,
	Ley Foon Tan <lftan@altera.com>
Subject: Re: [PATCH] of/fdt: fix error checking for earlycon address
Date: Wed, 28 Oct 2015 10:56:22 -0400	[thread overview]
Message-ID: <5630E216.908@hurleysoftware.com> (raw)
In-Reply-To: <CAL_JsqK6UJxvGt3_y5T+VMxpr7G=PCmQ_Nx0QW-EfeTHOCDMKQ@mail.gmail.com>

On 10/27/2015 04:46 PM, Rob Herring wrote:
> On Fri, Oct 23, 2015 at 8:15 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> Hi Masahiro,
>>
>> On 10/23/2015 07:47 AM, Masahiro Yamada wrote:
>>> fdt_translate_address() returns OF_BAD_ADDR on error.  It is defined as
>>> a u64 value, so the variable "addr" should be defined as u64 as well.
>>
>> Good catch.
>>
>> I would prefer if fdt_translate_address() returned resource_size_t (which
>> is the proper type for handling addresses as integers) and that type
>> was propagated through early_init_dt_scan_chosen_serial => of_setup_earlycon.
> 
> That can be problematic in the DT code. The size of resource_size_t
> can vary on 32-bit as it is based on phys_addr_t which in turn is
> based on CONFIG_LPAE setting. However, the address sizes in DT are
> fixed and may be 64-bit. So we stick with u64 in the DT code.

I think I see your point about fdt_translate_address(); you want it to
mirror of_translate_address(), yes?  If so, then I have no objection
to this patch and will mark it reviewed.

But the u64 value from fdt_translate_address() should be preserved
at least to the ioremap()/set_fixmap() in earlycon_map() .
I could roll that change into my earlycon series that moves
fdt_translate_address() into of_setup_earlycon(), thus preserving
the u64 value until it's assigned to a phys_addr_t.

Regards,
Peter Hurley

  parent reply	other threads:[~2015-10-28 14:56 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-23 11:47 [PATCH] of/fdt: fix error checking for earlycon address Masahiro Yamada
2015-10-23 13:15 ` Peter Hurley
     [not found]   ` <562A32EA.20808-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>
2015-10-27 20:46     ` Rob Herring
2015-10-27 20:46       ` Rob Herring
     [not found]       ` <CAL_JsqK6UJxvGt3_y5T+VMxpr7G=PCmQ_Nx0QW-EfeTHOCDMKQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-10-28 14:56         ` Peter Hurley [this message]
2015-10-28 14:56           ` Peter Hurley
2015-10-29  4:07 ` Rob Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5630E216.908@hurleysoftware.com \
    --to=peter-wagbzjegnqdsbiue7sb01tbpr1lh4cv8@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=lftan-EIB2kfCEclfQT0dZR+AlfA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.