On 09/27/2018 08:37 AM, Ang, Chee Hong wrote: > On Thu, 2018-09-27 at 08:21 +0200, Marek Vasut wrote: >> On 09/27/2018 07:08 AM, Ang, Chee Hong wrote: >>> >>> On Wed, 2018-09-26 at 16:53 +0200, Marek Vasut wrote: >>>> >>>> On 09/26/2018 11:03 AM, chee.hong....@intel.com wrote: >>>>> >>>>> >>>>> From: "Ang, Chee Hong" <chee.hong....@intel.com> >>>>> >>>>> Add a generic mailbox API for FPGA reconfig status which can be >>>>> called by others. This new function accepts 2 different mailbox >>>>> commands: CONFIG_STATUS or RECONFIG_STATUS. >>>> What "others" can call this ? >>> This is a common function used to check the readiness of the FPGA. >>> FPGA configuration drivers will need to call this to make sure >>> the FPGA configuration is successfully completed and running. >>> These FPGA configuration drivers will be submitted soon after this >>> patch. >> So this should be in drivers/fpga/ ? > Yes. There is a FPGA configuration driver under this folder. Will > submit soon. >> >> Also, don't add dead code, just submit this alongside the user of >> this code. > The reason I try to get this common function upstream is because my > colleague is trying to upstream socfpga dwmac driver which also need to > call this function to check whether the soft Ip running on FPGA for the > dwmac driver is up and running.
SoCFPGA dwmac support is already upstream. Can you explain why it needs to query the FPGA at all ? Is the whole dwmac in the FPGA ? Or is the dwmac just routed through FPGA ? > If I bundle this code alongside with my > FPGA configuration driver, it might take longer time to get into the > mainline. So this common function is blocking other to upstream his/her > code. I'm not really fond of accepting dead code, on which code I didn't even see and design that's unclear depends. >>>>> Signed-off-by: Ang, Chee Hong <chee.hong....@intel.com> >>>>> --- >>>>> arch/arm/mach-socfpga/include/mach/mailbox_s10.h | 3 +- >>>>> arch/arm/mach-socfpga/mailbox_s10.c | 48 >>>>> ++++++++++++++++++++++++ >>>>> 2 files changed, 50 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/arch/arm/mach-socfpga/include/mach/mailbox_s10.h >>>>> b/arch/arm/mach-socfpga/include/mach/mailbox_s10.h >>>>> index 81a609d..660df35 100644 >>>>> --- a/arch/arm/mach-socfpga/include/mach/mailbox_s10.h >>>>> +++ b/arch/arm/mach-socfpga/include/mach/mailbox_s10.h >>>>> @@ -140,5 +140,6 @@ int mbox_qspi_open(void); >>>>> #endif >>>>> >>>>> int mbox_reset_cold(void); >>>>> - >>>>> +int mbox_get_fpga_config_status(u32 cmd); >>>>> +int mbox_get_fpga_config_status_psci(u32 cmd); >>>>> #endif /* _MAILBOX_S10_H_ */ >>>>> diff --git a/arch/arm/mach-socfpga/mailbox_s10.c >>>>> b/arch/arm/mach- >>>>> socfpga/mailbox_s10.c >>>>> index 0d906c3..345485c 100644 >>>>> --- a/arch/arm/mach-socfpga/mailbox_s10.c >>>>> +++ b/arch/arm/mach-socfpga/mailbox_s10.c >>>>> @@ -342,6 +342,54 @@ int mbox_reset_cold(void) >>>>> return 0; >>>>> } >>>>> >>>>> +/* Accepted commands: CONFIG_STATUS or RECONFIG_STATUS */ >>>>> +static __always_inline int >>>>> mbox_get_fpga_config_status_common(u32 >>>>> cmd) >>>> Why __always_inline ? >>> This function is needed in 2 sections. Our u-boot run in normal >>> code >>> section and '__secure' section which mainly used for PSCI services. >>> Refer to the 'mbox_get_fpga_config_status()' and >>> 'mbox_get_fpga_config_status_psci()' below. These 2 functions run >>> in 2 >>> different sections and they need to call this function. >> But why does this need to be __always_inline ? The compiler can >> decide >> what's best. > This is dangerous. Let me explain in more details why we need this, > '__secure' section and normal '.code' section exist in different time. > '.code' section no longer valid once u-boot boot to OS, but '__secure' > section still exist after booting to OS because OS need to call PSCI > services to achieve certain things. We need to make sure both sections > contain the full code, therefore we need to enforce the compiler to > duplicate this piece of code to both sections as well. How does the __always_inline help with that ? [...] -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
