On Mon, Oct 05, 2020 at 08:37:13AM +0000, Zulkifli, Muhammad Husaini wrote: > Hi Sudeep, > > I am facing an error during sending yesterday. > I response again to your feedback as below > > >-----Original Message----- > >From: Sudeep Holla <sudeep.ho...@arm.com> > >Sent: Friday, October 2, 2020 10:51 PM > >To: Michal Simek <michal.si...@xilinx.com> > >Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulki...@intel.com>; > >Hunter, Adrian <adrian.hun...@intel.com>; ulf.hans...@linaro.org; linux- > >m...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux- > >ker...@vger.kernel.org; Raja Subramanian, Lakshmi Bai > ><lakshmi.bai.raja.subraman...@intel.com>; a...@arndb.de; Sudeep Holla > ><sudeep.ho...@arm.com>; Wan Mohamad, Wan Ahmad Zainie > ><wan.ahmad.zainie.wan.moha...@intel.com> > >Subject: Re: [PATCH v2 2/3] firmware: Keem Bay: Add support for Arm Trusted > >Firmware Service call > > > >Hi Michal, > > > >On Fri, Oct 02, 2020 at 03:53:33PM +0200, Michal Simek wrote: > >> Hi Sudeep, > >> > >> On 02. 10. 20 12:58, Sudeep Holla wrote: > >> > Hi Michal, > >> > > >> > On Fri, Oct 02, 2020 at 10:23:02AM +0200, Michal Simek wrote: > >> >> Hi Sudeep, > >> >> > >> >> On 01. 10. 20 17:35, Sudeep Holla wrote: > >> > > >> > [...] > >> > > >> >>> > >> >>> What are the other uses of this KEEMBAY_SIP_* ? > >> >>> For now I tend to move this to the driver making use of it using > >> >>> arm_smccc_1_1_invoke directly if possible. I don't see the need > >> >>> for this to be separate driver. But do let us know the features > >> >>> implemented in the firmware. If it is not v1.1+, reasons for not > >> >>> upgrading as you need v1.1 for some CPU errata implementation. > >> >> > >> >> This driver has been created based on my request to move it out the > >> >> mmc driver. It looks quite hacky to have arm_smccc_res and call > >> >> arm_smccc_smc() also with some IDs where it is visible that the > >> >> part of ID is just based on any spec. > >> > > >> > OK, driver is fine but no dt-bindings as it is discoverable. It can > >> > also be just a wrapper library instead as it needs no explicit > >> > initialisation like drivers to setup. > >> > >> I am fine with it. Do we have any example which we can point him to? > >> > > > >You seem to have figured that out already with SOC_ID example. > >That was quick I must say 😄. > > > >> > >> > > >> >> Also in v1 he is just calling SMC. But maybe there is going a need > >> >> to call HVC instead which is something what device driver shouldn't > >> >> decide that's why IMHO doing step via firmware driver is much better > >approach. > >> > > >> > Agreed and one must use arm_smccc_get_conduit or something similar. > >> > No additional bindings for each and ever platform and driver that > >> > uses SMCCC please. > >> > > >> >> Of course if there is a better/cleaner way how this should be done > >> >> I am happy to get more information about it. > >> >> > >> > > >> > Let me know what you think about my thoughts stated above. > >> > >> > >> I am fine with it. The key point is to have these sort it out because > >> I see that a lot of drivers just simply call that SMCs from drivers > >> which is IMHO wrong. > >> > > > >Sure, sorry I didn't express my concern properly. I want to avoid dt > >bindings for > >these and use the SMCCC discovery we have in place already if possible. > > > >If this driver had consumers in the DT and it needs to be represented in DT, > >it is > >a different story and I agree for need for a driver there. > >But I don't see one in this usecase. > > > Does it ok if I do some checking in arasan controller driver as below and > represented it in the DT of arasan,sdhci.yaml: > This is to ensure that for Keem Bay SOC specific, the firmware driver must be > consume. > > if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd")) { > struct device_node *dn; > struct gpio_desc *uhs; > > dn = of_find_node_by_name(NULL, "keembay_firmware");
You have keembay_sd_voltage_selection function as Michal prefers, I have no objections for that. But please no keembay_firmware node in DT. You can implement this as a driver or simple smccc based function library without DT node using SMCCC get_version. I hope the firmware gives error for unimplemented FIDs, thereby eliminating the need for any DT node or config option. -- Regards, Sudeep