From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vince Hsu Subject: Re: [RFC PATCH 2/9] memory: tegra: add mc flush support Date: Tue, 3 Mar 2015 16:59:46 +0800 Message-ID: <54F57802.6010101@nvidia.com> References: <1421216372-8025-1-git-send-email-vinceh@nvidia.com> <1421216372-8025-3-git-send-email-vinceh@nvidia.com> <54F42549.5040202@nvidia.com> <54F56C26.1020202@nvidia.com> <54F56E4E.9070004@nvidia.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-tegra-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Alexandre Courbot Cc: Thierry Reding , Peter De Schrijver , Stephen Warren , "linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" List-Id: linux-tegra@vger.kernel.org On 03/03/2015 04:31 PM, Alexandre Courbot wrote: > On Tue, Mar 3, 2015 at 5:18 PM, Vince Hsu wrote: >> On 03/03/2015 04:14 PM, Alexandre Courbot wrote: >>> On Tue, Mar 3, 2015 at 5:09 PM, Vince Hsu wrote: >>>> On 03/03/2015 04:03 PM, Alexandre Courbot wrote: >>>>> On Mon, Mar 2, 2015 at 6:29 PM, Alexandre Courbot >>>>> wrote: >>>>>>>> These functions are identical, excepted for the callback they are >>>>>>>> invoking. Could you merge the common part into a function that >>>>>>>> returns >>>>>>>> the right client to call the callback on, or ERR_PTR(-EINVAL) in case >>>>>>>> of failure? >>>>>>> I couldn't think of a clever way to do this. Any ideas? :) >>>>>> How about something like this (warning: might now be that great, >>>>>> untested): >>>>>> >>>>>> /* Have this in your .h and use it in your tegra_mc_ops struct */ >>>>>> typedef int (*mc_op)(struct tegra_mc *mc, >>>>>> const struct tegra_mc_hotreset *hotreset) >>>>> This type should be named tegra_mc_op, since the header that defines >>>>> it is in include/linux. >>>> Can we just leave it in this C file? I see no reason to place it in some >>>> other >>>> header file. :) >>> Can you also move tegra_mc_ops's definition into the C file? If so I >>> agree. Otherwise I prefer to make sure that the same type is used >>> everywhere. >> No. Let's keep that in the include/soc/tegra/mc.h. > Actually, couldn't some of these definitions be moved into > drivers/memory/tegra/mc.h? I don't see any reference to e.g. struct > tegra_mc_soc outside of drivers/memory/tegra/. These doesn't seem to > be a need to make these available to everybody. It seems all the definitions were in the drivers/memory/tegra/tegra-mc.h. And then due to some reason, Thierry moved them under include/soc/tegra. And we do need the struct tegra_mc_soc in the include/soc/tegra/tegra/mc.h because struct tegra_mc refers to it and iommu/tegra-smmu.c refers to struct tegra_mc. Thanks, Vince