Hi Uffe,
> -----Original Message----- > From: Ulf Hansson <[email protected]> > Sent: Wednesday, June 19, 2019 8:11 PM > To: Manish Narani <[email protected]> > Cc: Michal Simek <[email protected]>; Rob Herring <[email protected]>; > Mark Rutland <[email protected]>; Adrian Hunter > <[email protected]>; Rajan Vaja <[email protected]>; Jolly Shah > <[email protected]>; Nava kishore Manne <[email protected]>; Olof > Johansson <[email protected]>; [email protected]; DTML > <[email protected]>; Linux Kernel Mailing List <linux- > [email protected]>; Linux ARM <[email protected]> > Subject: Re: [PATCH 3/3] mmc: sdhci-of-arasan: Add support for ZynqMP > Platform Tap Delays Setup > > On Tue, 18 Jun 2019 at 06:59, Manish Narani <[email protected]> wrote: > > > > Hi Uffe, > > > > Thanks for the review. Please find my comments below. > > > > > -----Original Message----- > > > From: Ulf Hansson <[email protected]> > > > Sent: Monday, June 17, 2019 8:29 PM > > > To: Michal Simek <[email protected]> > > > Cc: Manish Narani <[email protected]>; Rob Herring > > > <[email protected]>; Mark Rutland <[email protected]>; Adrian > > > Hunter <[email protected]>; Rajan Vaja <[email protected]>; Jolly > > > Shah <[email protected]>; Nava kishore Manne <[email protected]>; > Olof > > > Johansson <[email protected]>; [email protected]; DTML > > > <[email protected]>; Linux Kernel Mailing List <linux- > > > [email protected]>; Linux ARM <linux-arm- > [email protected]> > > > Subject: Re: [PATCH 3/3] mmc: sdhci-of-arasan: Add support for ZynqMP > > > Platform Tap Delays Setup > > > > > > [...] > > > > > > > >> > > > > >> > > > > >>> In regards to the mmc data part, I suggest to drop the > > > > >>> ->set_tap_delay() callback, but rather use a boolean flag to > > > > >>> indicate > > > > >>> whether clock phases needs to be changed for the variant. > > > > >>> Potentially > > > > >>> that could even be skipped and instead call clk_set_phase() > > > > >>> unconditionally, as the clock core deals fine with clock providers > > > > >>> that doesn't support the ->set_phase() callback. > > > > In the current implementation, I am taking care of both the input and > > output clock delays with the single clock (which is output clock) > > registration > > and differentiating these tap delays based on their values > > (<256 then input delay and >= 256 then output delay), because that is > > zynqmp specific. If we want to make this generic, we may need to > > register 'another' clock which will be there as an input (sampling) clock > > and then we can make this 'clk_set_phase()' be called unconditionally > > each for both the clocks and let the platforms handle their clock part. > > What's your take on this? > > Not sure exactly what you are suggesting, but my gut feeling says it > sounds good. > > How is tap delays managed for both the input clock and the output > clock? Is some managed by the clock provider (which is probably > firmware in your case) and some managed by the MMC controller? Yes, for the existing "xlnx,zynqmp-8.9a" controller, the tap delays will be managed by the firmware, however in the upcoming "xlnx,versal-8.9a" variant the tap delays will be managed by the MMC controller itself. I will include the Versal one in the next series of patches. Thanks, Manish
