From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH v4 1/1] Support Odroid-XU board (Exynos 5410)
Date: Tue, 26 Aug 2014 18:34:58 +0100
Message-ID: <1409074498.28009.9.camel@citrix.com>
References: <1408490361-308-1-git-send-email-suriyan.r@gmail.com>
	<1408659683.15186.3.camel@citrix.com>
	<CANoR_ODiKr3X2BsY-Vha=Q-nBQeWuK9VWHA+dV0u_B_WrsUbSQ@mail.gmail.com>
	<CANoR_OCER=8mx_E58D7y6JLK5mf+fC3pUW1-PF5OwNja88_RWg@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <CANoR_OCER=8mx_E58D7y6JLK5mf+fC3pUW1-PF5OwNja88_RWg@mail.gmail.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Suriyan Ramasami <suriyan.r@gmail.com>
Cc: keir@xen.org, Julien Grall <julien.grall@linaro.org>, Tim Deegan <tim@xen.org>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, Jan Beulich <jbeulich@suse.com>, ian.jackson@citrix.com
List-Id: xen-devel@lists.xenproject.org

On Mon, 2014-08-25 at 15:06 -0700, Suriyan Ramasami wrote:
> On Mon, Aug 25, 2014 at 2:45 PM, Suriyan Ramasami <suriyan.r@gmail.com> wrote:
> > On Thu, Aug 21, 2014 at 3:21 PM, Ian Campbell <ian.campbell@citrix.com> wrote:
> >> On Tue, 2014-08-19 at 16:19 -0700, Suriyan Ramasami wrote:
> >>> @@ -64,11 +70,11 @@ static int exynos5_specific_mapping(struct domain *d)
> >>>      return 0;
> >>>  }
> >>>
> >>> -static int __init exynos5_smp_init(void)
> >>> +static int __init exynos_smp_init(paddr_t pa_sysram)
> >>> [...]
> >>
> >>> +static int __init exynos5250_smp_init(void)
> >>> [...]
> >>> +static int __init exynos5_smp_init(void)
> >>
> >> I'm not sure I grok the intending meaning/distinction of exynos5_* vs
> >> exynos5250_*. It *looks* like you are using exynos5_* for the 5410
> >> specific stuff, but I'm not sure (and I'm half suspecting that you are
> >> using it for both 5410 specific and more general things...).
> >>
> >> If that is the case then you really ought to be using exynos5410_* for
> >> stuff which is specific to that chip.
> >>
> >> Perhaps exynos5xxx_* would be less confusing for the generic stuff, but
> >> just avoiding using exynos5_* for 5410 specific stuff would be
> >> sufficient.
> >>
> > Thank you Ian for the review and suggestion. I shall use exynos5xxx_*
> > for generic and exynos5410_* and exynos5250_* names in my next
> > iteration.
> >
> I just looked at the code again. I do find that it I have consistently
> used exynos5_* for generic functions and exynos5410_* for 5410 specifc
> and exynos5250_* for 5250 specific functions. The function
> exynos5_smp_init(void) was moved down the file, and that is possibly
> causing some of this confusion (from a diff perspective).

Yes, that could well be what confused me, sorry about that.

If you can avoid moving the function, then great, otherwise then don't
worry.

> The exynos_* functions are helpers that have the same name as used in
> the linux tree to help in future backports as suggested by Julien.
> 
> Please let me know if this is OK.

>>From what you've said here it sounds like it, I'll take another look at
the patch, I'm terribly backlogged at the moment (vacation followed by
conference travel...)

Ian.