| Message ID | 20200124115252.15712-3-faiz_abbas@ti.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | Lokesh Vutla |
| Headers | show |
| Series | Add Support for eMMC boot in AM65x and J721e | expand |
On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote: > > Add an init() API for platform specific init() operations. Could you describe why this cannot be done in the probe callback? It's not easily visible as the function you changed (mmc_get_op_cond) doesn't even have a comment to describe what it does... In general, I think commit messages could be more detailed than one line. If only to make it easier in the future to recap why things have been done. > > Signed-off-by: Faiz Abbas <faiz_abbas@ti.com> > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> > --- > drivers/mmc/mmc.c | 13 ++++++------- > include/mmc.h | 7 +++++++ > 2 files changed, 13 insertions(+), 7 deletions(-) > > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > index d43983d4a6..50df8c8626 100644 > --- a/drivers/mmc/mmc.c > +++ b/drivers/mmc/mmc.c > @@ -2787,14 +2787,13 @@ int mmc_get_op_cond(struct mmc *mmc) > } > if (err) > return err; > - > #if CONFIG_IS_ENABLED(DM_MMC) > - /* The device has already been probed ready for use */ > -#else > - /* made sure it's not NULL earlier */ > - err = mmc->cfg->ops->init(mmc); > - if (err) > - return err; You're removing the init code for non-DM MMC here and did not describe it in the commit message. Is this change intended at all? Regards, Simon > + struct dm_mmc_ops *ops = mmc_get_ops(mmc->dev); > + if (ops->init) { > + err = ops->init(mmc->dev); > + if (err) > + return err; > + } > #endif > mmc->ddr_mode = 0; > > diff --git a/include/mmc.h b/include/mmc.h > index 2f21dbf1b7..6aef125f25 100644 > --- a/include/mmc.h > +++ b/include/mmc.h > @@ -406,6 +406,13 @@ struct mmc; > > #if CONFIG_IS_ENABLED(DM_MMC) > struct dm_mmc_ops { > + /** > + * init() - platform specific initialization for the host controller > + * > + * @dev: Device to init > + * @return 0 if Ok, -ve if error > + */ > + int (*init)(struct udevice *dev); > /** > * send_cmd() - Send a command to the MMC device > * > -- > 2.19.2 >
Forgot to ask: why isn't the subsystem maintainer on CC? If you would use patman to send patches or at least the get_maintainer script, he would have been... Regards, Simon On Wed, Jan 29, 2020 at 9:03 AM Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote: > > On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote: > > > > Add an init() API for platform specific init() operations. > > Could you describe why this cannot be done in the probe callback? It's not > easily visible as the function you changed (mmc_get_op_cond) doesn't even have > a comment to describe what it does... > > In general, I think commit messages could be more detailed than one line. If > only to make it easier in the future to recap why things have been done. > > > > > Signed-off-by: Faiz Abbas <faiz_abbas@ti.com> > > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> > > --- > > drivers/mmc/mmc.c | 13 ++++++------- > > include/mmc.h | 7 +++++++ > > 2 files changed, 13 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > > index d43983d4a6..50df8c8626 100644 > > --- a/drivers/mmc/mmc.c > > +++ b/drivers/mmc/mmc.c > > @@ -2787,14 +2787,13 @@ int mmc_get_op_cond(struct mmc *mmc) > > } > > if (err) > > return err; > > - > > #if CONFIG_IS_ENABLED(DM_MMC) > > - /* The device has already been probed ready for use */ > > -#else > > - /* made sure it's not NULL earlier */ > > - err = mmc->cfg->ops->init(mmc); > > - if (err) > > - return err; > > You're removing the init code for non-DM MMC here and did not describe it in > the commit message. Is this change intended at all? > > Regards, > Simon > > > + struct dm_mmc_ops *ops = mmc_get_ops(mmc->dev); > > + if (ops->init) { > > + err = ops->init(mmc->dev); > > + if (err) > > + return err; > > + } > > #endif > > mmc->ddr_mode = 0; > > > > diff --git a/include/mmc.h b/include/mmc.h > > index 2f21dbf1b7..6aef125f25 100644 > > --- a/include/mmc.h > > +++ b/include/mmc.h > > @@ -406,6 +406,13 @@ struct mmc; > > > > #if CONFIG_IS_ENABLED(DM_MMC) > > struct dm_mmc_ops { > > + /** > > + * init() - platform specific initialization for the host controller > > + * > > + * @dev: Device to init > > + * @return 0 if Ok, -ve if error > > + */ > > + int (*init)(struct udevice *dev); > > /** > > * send_cmd() - Send a command to the MMC device > > * > > -- > > 2.19.2 > >
Hi Simon, On 29/01/20 1:33 pm, Simon Goldschmidt wrote: > On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote: >> >> Add an init() API for platform specific init() operations. > > Could you describe why this cannot be done in the probe callback? It's not > easily visible as the function you changed (mmc_get_op_cond) doesn't even have > a comment to describe what it does... The reason is detailed in 06/10 patch description. probe() is always called for all MMC instances. I only want to switch on power (by calling sdhci_init()) and suffer the 1 second wait time when there is actually a card in the slot and user wants to access it. > > In general, I think commit messages could be more detailed than one line. If > only to make it easier in the future to recap why things have been done. > You're right. I will add a more detailed patch description in v2. Thanks, Faiz
Hi, +Lokesh, Tom On 29/01/20 1:37 pm, Simon Goldschmidt wrote: > Forgot to ask: why isn't the subsystem maintainer on CC? > If you would use patman to send patches or at least the get_maintainer script, > he would have been... > I did use get_maintainer for my send-email CC list but everyone other than Michal seems to have been dropped. Here is an excerpt from the email header I received: From: Faiz Abbas <faiz_abbas@ti.com> To: <u-boot@lists.denx.de> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, <lokeshvutla@ti.com>, <peng.fan@nxp.com>, <faiz_abbas@ti.com> Subject: [PATCH v2 02/10] mmc: Add init() API Date: Fri, 24 Jan 2020 17:22:44 +0530 But in the patchworks and in your reply, only Michal is remaining: https://patchwork.ozlabs.org/patch/1228781/ Michal, What do you see in your message header? Does it have other people copied? Thanks, Faiz
On 30. 01. 20 16:03, Faiz Abbas wrote: > Hi, > > +Lokesh, Tom > > On 29/01/20 1:37 pm, Simon Goldschmidt wrote: >> Forgot to ask: why isn't the subsystem maintainer on CC? >> If you would use patman to send patches or at least the get_maintainer script, >> he would have been... >> > > I did use get_maintainer for my send-email CC list but everyone other > than Michal seems to have been dropped. Here is an excerpt from the > email header I received: > > From: Faiz Abbas <faiz_abbas@ti.com> > To: <u-boot@lists.denx.de> > CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, <lokeshvutla@ti.com>, > <peng.fan@nxp.com>, <faiz_abbas@ti.com> > Subject: [PATCH v2 02/10] mmc: Add init() API > Date: Fri, 24 Jan 2020 17:22:44 +0530 > > > But in the patchworks and in your reply, only Michal is remaining: > https://patchwork.ozlabs.org/patch/1228781/ > > Michal, > > What do you see in your message header? Does it have other people copied? [u-boot]$ ./scripts/get_maintainer.pl -f drivers/mmc/mmc.c Peng Fan <peng.fan@nxp.com> (maintainer:MMC) u-boot@lists.denx.de (open list) I see Peng there. Thanks, Michal
Faiz Abbas <faiz_abbas@ti.com> schrieb am Do., 30. Jan. 2020, 16:01: > Hi, > > +Lokesh, Tom > > On 29/01/20 1:37 pm, Simon Goldschmidt wrote: > > Forgot to ask: why isn't the subsystem maintainer on CC? > > If you would use patman to send patches or at least the get_maintainer > script, > > he would have been... > > > > I did use get_maintainer for my send-email CC list but everyone other > than Michal seems to have been dropped. Here is an excerpt from the > email header I received: > > From: Faiz Abbas <faiz_abbas@ti.com> > To: <u-boot@lists.denx.de> > CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, <lokeshvutla@ti.com>, > <peng.fan@nxp.com>, <faiz_abbas@ti.com> > Subject: [PATCH v2 02/10] mmc: Add init() API > Date: Fri, 24 Jan 2020 17:22:44 +0530 > > > But in the patchworks and in your reply, only Michal is remaining: > https://patchwork.ozlabs.org/patch/1228781/ I hit reply all in the gmail web interface. My header only shows Michal. Regards, Simon > > Michal, > > What do you see in your message header? Does it have other people copied? > Thanks, > Faiz >
Hi Michal, On 30/01/20 8:37 pm, Michal Simek wrote: > On 30. 01. 20 16:03, Faiz Abbas wrote: >> Hi, >> >> +Lokesh, Tom >> >> On 29/01/20 1:37 pm, Simon Goldschmidt wrote: >>> Forgot to ask: why isn't the subsystem maintainer on CC? >>> If you would use patman to send patches or at least the get_maintainer script, >>> he would have been... >>> >> >> I did use get_maintainer for my send-email CC list but everyone other >> than Michal seems to have been dropped. Here is an excerpt from the >> email header I received: >> >> From: Faiz Abbas <faiz_abbas@ti.com> >> To: <u-boot@lists.denx.de> >> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, <lokeshvutla@ti.com>, >> <peng.fan@nxp.com>, <faiz_abbas@ti.com> >> Subject: [PATCH v2 02/10] mmc: Add init() API >> Date: Fri, 24 Jan 2020 17:22:44 +0530 >> >> >> But in the patchworks and in your reply, only Michal is remaining: >> https://patchwork.ozlabs.org/patch/1228781/ >> >> Michal, >> >> What do you see in your message header? Does it have other people copied? > > [u-boot]$ ./scripts/get_maintainer.pl -f drivers/mmc/mmc.c > Peng Fan <peng.fan@nxp.com> (maintainer:MMC) > u-boot@lists.denx.de (open list) > > I see Peng there. > You misunderstood. I am not asking if you see Peng in the get_maintainer output. Do you see him CC'd in the original patch email? Thanks, Faiz
On 30. 01. 20 16:14, Faiz Abbas wrote: > Hi Michal, > > On 30/01/20 8:37 pm, Michal Simek wrote: >> On 30. 01. 20 16:03, Faiz Abbas wrote: >>> Hi, >>> >>> +Lokesh, Tom >>> >>> On 29/01/20 1:37 pm, Simon Goldschmidt wrote: >>>> Forgot to ask: why isn't the subsystem maintainer on CC? >>>> If you would use patman to send patches or at least the get_maintainer script, >>>> he would have been... >>>> >>> >>> I did use get_maintainer for my send-email CC list but everyone other >>> than Michal seems to have been dropped. Here is an excerpt from the >>> email header I received: >>> >>> From: Faiz Abbas <faiz_abbas@ti.com> >>> To: <u-boot@lists.denx.de> >>> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, <lokeshvutla@ti.com>, >>> <peng.fan@nxp.com>, <faiz_abbas@ti.com> >>> Subject: [PATCH v2 02/10] mmc: Add init() API >>> Date: Fri, 24 Jan 2020 17:22:44 +0530 >>> >>> >>> But in the patchworks and in your reply, only Michal is remaining: >>> https://patchwork.ozlabs.org/patch/1228781/ >>> >>> Michal, >>> >>> What do you see in your message header? Does it have other people copied? >> >> [u-boot]$ ./scripts/get_maintainer.pl -f drivers/mmc/mmc.c >> Peng Fan <peng.fan@nxp.com> (maintainer:MMC) >> u-boot@lists.denx.de (open list) >> >> I see Peng there. >> > > You misunderstood. I am not asking if you see Peng in the get_maintainer > output. Do you see him CC'd in the original patch email? Nope. I can't see him there. M
On Thu, Jan 30, 2020 at 04:30:54PM +0100, Michal Simek wrote: > On 30. 01. 20 16:14, Faiz Abbas wrote: > > Hi Michal, > > > > On 30/01/20 8:37 pm, Michal Simek wrote: > >> On 30. 01. 20 16:03, Faiz Abbas wrote: > >>> Hi, > >>> > >>> +Lokesh, Tom > >>> > >>> On 29/01/20 1:37 pm, Simon Goldschmidt wrote: > >>>> Forgot to ask: why isn't the subsystem maintainer on CC? > >>>> If you would use patman to send patches or at least the get_maintainer script, > >>>> he would have been... > >>>> > >>> > >>> I did use get_maintainer for my send-email CC list but everyone other > >>> than Michal seems to have been dropped. Here is an excerpt from the > >>> email header I received: > >>> > >>> From: Faiz Abbas <faiz_abbas@ti.com> > >>> To: <u-boot@lists.denx.de> > >>> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, <lokeshvutla@ti.com>, > >>> <peng.fan@nxp.com>, <faiz_abbas@ti.com> > >>> Subject: [PATCH v2 02/10] mmc: Add init() API > >>> Date: Fri, 24 Jan 2020 17:22:44 +0530 > >>> > >>> > >>> But in the patchworks and in your reply, only Michal is remaining: > >>> https://patchwork.ozlabs.org/patch/1228781/ > >>> > >>> Michal, > >>> > >>> What do you see in your message header? Does it have other people copied? > >> > >> [u-boot]$ ./scripts/get_maintainer.pl -f drivers/mmc/mmc.c > >> Peng Fan <peng.fan@nxp.com> (maintainer:MMC) > >> u-boot@lists.denx.de (open list) > >> > >> I see Peng there. > >> > > > > You misunderstood. I am not asking if you see Peng in the get_maintainer > > output. Do you see him CC'd in the original patch email? > > Nope. I can't see him there. Wolfgang, is there some mailman setting that needs tweaking or looking at here? Thanks!
Tom Rini <trini@konsulko.com> schrieb am Do., 30. Jan. 2020, 16:32: > On Thu, Jan 30, 2020 at 04:30:54PM +0100, Michal Simek wrote: > > On 30. 01. 20 16:14, Faiz Abbas wrote: > > > Hi Michal, > > > > > > On 30/01/20 8:37 pm, Michal Simek wrote: > > >> On 30. 01. 20 16:03, Faiz Abbas wrote: > > >>> Hi, > > >>> > > >>> +Lokesh, Tom > > >>> > > >>> On 29/01/20 1:37 pm, Simon Goldschmidt wrote: > > >>>> Forgot to ask: why isn't the subsystem maintainer on CC? > > >>>> If you would use patman to send patches or at least the > get_maintainer script, > > >>>> he would have been... > > >>>> > > >>> > > >>> I did use get_maintainer for my send-email CC list but everyone other > > >>> than Michal seems to have been dropped. Here is an excerpt from the > > >>> email header I received: > > >>> > > >>> From: Faiz Abbas <faiz_abbas@ti.com> > > >>> To: <u-boot@lists.denx.de> > > >>> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, < > lokeshvutla@ti.com>, > > >>> <peng.fan@nxp.com>, <faiz_abbas@ti.com> > > >>> Subject: [PATCH v2 02/10] mmc: Add init() API > > >>> Date: Fri, 24 Jan 2020 17:22:44 +0530 > > >>> > > >>> > > >>> But in the patchworks and in your reply, only Michal is remaining: > > >>> https://patchwork.ozlabs.org/patch/1228781/ > > >>> > > >>> Michal, > > >>> > > >>> What do you see in your message header? Does it have other people > copied? > > >> > > >> [u-boot]$ ./scripts/get_maintainer.pl -f drivers/mmc/mmc.c > > >> Peng Fan <peng.fan@nxp.com> (maintainer:MMC) > > >> u-boot@lists.denx.de (open list) > > >> > > >> I see Peng there. > > >> > > > > > > You misunderstood. I am not asking if you see Peng in the > get_maintainer > > > output. Do you see him CC'd in the original patch email? > > > > Nope. I can't see him there. > > Wolfgang, is there some mailman setting that needs tweaking or looking > at here? Thanks! > Can this be a mailman issue? If Michal was CCed, is mailman involved in the way to him? I would have thought that mail got delivered directly. Regards, Simon
On Thu, Jan 30, 2020 at 04:35:40PM +0100, Simon Goldschmidt wrote: > Tom Rini <trini@konsulko.com> schrieb am Do., 30. Jan. 2020, 16:32: > > > On Thu, Jan 30, 2020 at 04:30:54PM +0100, Michal Simek wrote: > > > On 30. 01. 20 16:14, Faiz Abbas wrote: > > > > Hi Michal, > > > > > > > > On 30/01/20 8:37 pm, Michal Simek wrote: > > > >> On 30. 01. 20 16:03, Faiz Abbas wrote: > > > >>> Hi, > > > >>> > > > >>> +Lokesh, Tom > > > >>> > > > >>> On 29/01/20 1:37 pm, Simon Goldschmidt wrote: > > > >>>> Forgot to ask: why isn't the subsystem maintainer on CC? > > > >>>> If you would use patman to send patches or at least the > > get_maintainer script, > > > >>>> he would have been... > > > >>>> > > > >>> > > > >>> I did use get_maintainer for my send-email CC list but everyone other > > > >>> than Michal seems to have been dropped. Here is an excerpt from the > > > >>> email header I received: > > > >>> > > > >>> From: Faiz Abbas <faiz_abbas@ti.com> > > > >>> To: <u-boot@lists.denx.de> > > > >>> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, < > > lokeshvutla@ti.com>, > > > >>> <peng.fan@nxp.com>, <faiz_abbas@ti.com> > > > >>> Subject: [PATCH v2 02/10] mmc: Add init() API > > > >>> Date: Fri, 24 Jan 2020 17:22:44 +0530 > > > >>> > > > >>> > > > >>> But in the patchworks and in your reply, only Michal is remaining: > > > >>> https://patchwork.ozlabs.org/patch/1228781/ > > > >>> > > > >>> Michal, > > > >>> > > > >>> What do you see in your message header? Does it have other people > > copied? > > > >> > > > >> [u-boot]$ ./scripts/get_maintainer.pl -f drivers/mmc/mmc.c > > > >> Peng Fan <peng.fan@nxp.com> (maintainer:MMC) > > > >> u-boot@lists.denx.de (open list) > > > >> > > > >> I see Peng there. > > > >> > > > > > > > > You misunderstood. I am not asking if you see Peng in the > > get_maintainer > > > > output. Do you see him CC'd in the original patch email? > > > > > > Nope. I can't see him there. > > > > Wolfgang, is there some mailman setting that needs tweaking or looking > > at here? Thanks! > > > > Can this be a mailman issue? If Michal was CCed, is mailman involved in the > way to him? I would have thought that mail got delivered directly. I was thinking about the setting on if you get your own messages / ones you're on CC to or not, and if that was the copy say in Michal's inbox or U-Boot folder.
Dear Tom, In message <20200130153216.GX13379@bill-the-cat> you wrote: > > > > You misunderstood. I am not asking if you see Peng in the get_maintainer > > > output. Do you see him CC'd in the original patch email? > > > > Nope. I can't see him there. > > Wolfgang, is there some mailman setting that needs tweaking or looking > at here? Thanks! I'm not sure. I cannot see any trae of the Cc: s at list.denx.de, as this is handled by the poster's MTA. I can only see the posting as it arrived at the list server, and this has just From: Faiz Abbas <faiz_abbas@ti.com> To: <u-boot@lists.denx.de> Cc: michal.simek@xilinx.com It would be interesting to check the MTA los on the sender side if the Cc:s were actually sent... Best regards, Wolfgang Denk
Dear Simon, In message <CAAh8qswkmJhpgSZx4KcGiXrEugS47M+_VB7Nkcs27kpDLpVcAQ@mail.gmail.com> you wrote: > > Can this be a mailman issue? If Michal was CCed, is mailman involved in the > way to him? I would have thought that mail got delivered directly. Correct, it's the sender's MTA who handles the action transmission. The question here is if there was actually more than the single address in the Cc: header when the message to the list got sent. For example, it would be interesting to know if any of the people who were supposed to be on the Cc: list (but now don't show up) got a mesage directly, or only through the mailing list. Best regards, Wolfgang Denk
Dear Tom, In message <20200130153827.GY13379@bill-the-cat> you wrote: > > > way to him? I would have thought that mail got delivered directly. > > I was thinking about the setting on if you get your own messages / ones > you're on CC to or not, and if that was the copy say in Michal's inbox > or U-Boot folder. Yes, but thisis a secondaryquestion here. The more interesting one (especially for patchwork etc.) is where the Cc: header lost the additional addresses - was it at the sender's MUA, the sender's MTA or on lists.denx.de ; I don't really trust mailman if it's getting dark outside, but I have no good way to check this. Best regards, Wolfgang Denk
> Subject: Re: [PATCH v2 02/10] mmc: Add init() API > > Hi Simon, > > On 29/01/20 1:33 pm, Simon Goldschmidt wrote: > > On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote: > >> > >> Add an init() API for platform specific init() operations. > > > > Could you describe why this cannot be done in the probe callback? It's > > not easily visible as the function you changed (mmc_get_op_cond) > > doesn't even have a comment to describe what it does... > > The reason is detailed in 06/10 patch description. probe() is always called for > all MMC instances. I only want to switch on power (by calling > sdhci_init()) and suffer the 1 second wait time when there is actually a card in > the slot and user wants to access it. > > > > In general, I think commit messages could be more detailed than one > > line. If only to make it easier in the future to recap why things have been > done. > > > > You're right. I will add a more detailed patch description in v2. This patch is v2. Please v3. Could a quirk be added in probe and if card not there, just bypass? Or you want quick boot to avoid probe all the controllers? Regards, Peng. > > Thanks, > Faiz
Michal, On 30/01/20 9:08 PM, Tom Rini wrote: > On Thu, Jan 30, 2020 at 04:35:40PM +0100, Simon Goldschmidt wrote: >> Tom Rini <trini@konsulko.com> schrieb am Do., 30. Jan. 2020, 16:32: >> >>> On Thu, Jan 30, 2020 at 04:30:54PM +0100, Michal Simek wrote: >>>> On 30. 01. 20 16:14, Faiz Abbas wrote: >>>>> Hi Michal, >>>>> >>>>> On 30/01/20 8:37 pm, Michal Simek wrote: >>>>>> On 30. 01. 20 16:03, Faiz Abbas wrote: >>>>>>> Hi, >>>>>>> >>>>>>> +Lokesh, Tom >>>>>>> >>>>>>> On 29/01/20 1:37 pm, Simon Goldschmidt wrote: >>>>>>>> Forgot to ask: why isn't the subsystem maintainer on CC? >>>>>>>> If you would use patman to send patches or at least the >>> get_maintainer script, >>>>>>>> he would have been... >>>>>>>> >>>>>>> >>>>>>> I did use get_maintainer for my send-email CC list but everyone other >>>>>>> than Michal seems to have been dropped. Here is an excerpt from the >>>>>>> email header I received: >>>>>>> >>>>>>> From: Faiz Abbas <faiz_abbas@ti.com> >>>>>>> To: <u-boot@lists.denx.de> >>>>>>> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, < >>> lokeshvutla@ti.com>, >>>>>>> <peng.fan@nxp.com>, <faiz_abbas@ti.com> >>>>>>> Subject: [PATCH v2 02/10] mmc: Add init() API >>>>>>> Date: Fri, 24 Jan 2020 17:22:44 +0530 >>>>>>> >>>>>>> >>>>>>> But in the patchworks and in your reply, only Michal is remaining: >>>>>>> https://patchwork.ozlabs.org/patch/1228781/ >>>>>>> >>>>>>> Michal, >>>>>>> >>>>>>> What do you see in your message header? Does it have other people >>> copied? >>>>>> >>>>>> [u-boot]$ ./scripts/get_maintainer.pl -f drivers/mmc/mmc.c >>>>>> Peng Fan <peng.fan@nxp.com> (maintainer:MMC) >>>>>> u-boot@lists.denx.de (open list) >>>>>> >>>>>> I see Peng there. >>>>>> >>>>> >>>>> You misunderstood. I am not asking if you see Peng in the >>> get_maintainer >>>>> output. Do you see him CC'd in the original patch email? >>>> >>>> Nope. I can't see him there. >>> >>> Wolfgang, is there some mailman setting that needs tweaking or looking >>> at here? Thanks! >>> >> >> Can this be a mailman issue? If Michal was CCed, is mailman involved in the >> way to him? I would have thought that mail got delivered directly. > > I was thinking about the setting on if you get your own messages / ones > you're on CC to or not, and if that was the copy say in Michal's inbox > or U-Boot folder. Can you confirm how you received the message, directly or through the list? The TI e-mail team says the CCs were not removed prior to leaving TI. Thanks, Sekhar
On 03. 02. 20 7:04, Sekhar Nori wrote: > Michal, > > On 30/01/20 9:08 PM, Tom Rini wrote: >> On Thu, Jan 30, 2020 at 04:35:40PM +0100, Simon Goldschmidt wrote: >>> Tom Rini <trini@konsulko.com> schrieb am Do., 30. Jan. 2020, 16:32: >>> >>>> On Thu, Jan 30, 2020 at 04:30:54PM +0100, Michal Simek wrote: >>>>> On 30. 01. 20 16:14, Faiz Abbas wrote: >>>>>> Hi Michal, >>>>>> >>>>>> On 30/01/20 8:37 pm, Michal Simek wrote: >>>>>>> On 30. 01. 20 16:03, Faiz Abbas wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> +Lokesh, Tom >>>>>>>> >>>>>>>> On 29/01/20 1:37 pm, Simon Goldschmidt wrote: >>>>>>>>> Forgot to ask: why isn't the subsystem maintainer on CC? >>>>>>>>> If you would use patman to send patches or at least the >>>> get_maintainer script, >>>>>>>>> he would have been... >>>>>>>>> >>>>>>>> >>>>>>>> I did use get_maintainer for my send-email CC list but everyone other >>>>>>>> than Michal seems to have been dropped. Here is an excerpt from the >>>>>>>> email header I received: >>>>>>>> >>>>>>>> From: Faiz Abbas <faiz_abbas@ti.com> >>>>>>>> To: <u-boot@lists.denx.de> >>>>>>>> CC: <dannenberg@ti.com>, <michal.simek@xilinx.com>, < >>>> lokeshvutla@ti.com>, >>>>>>>> <peng.fan@nxp.com>, <faiz_abbas@ti.com> >>>>>>>> Subject: [PATCH v2 02/10] mmc: Add init() API >>>>>>>> Date: Fri, 24 Jan 2020 17:22:44 +0530 >>>>>>>> >>>>>>>> >>>>>>>> But in the patchworks and in your reply, only Michal is remaining: >>>>>>>> https://patchwork.ozlabs.org/patch/1228781/ >>>>>>>> >>>>>>>> Michal, >>>>>>>> >>>>>>>> What do you see in your message header? Does it have other people >>>> copied? >>>>>>> >>>>>>> [u-boot]$ ./scripts/get_maintainer.pl -f drivers/mmc/mmc.c >>>>>>> Peng Fan <peng.fan@nxp.com> (maintainer:MMC) >>>>>>> u-boot@lists.denx.de (open list) >>>>>>> >>>>>>> I see Peng there. >>>>>>> >>>>>> >>>>>> You misunderstood. I am not asking if you see Peng in the >>>> get_maintainer >>>>>> output. Do you see him CC'd in the original patch email? >>>>> >>>>> Nope. I can't see him there. >>>> >>>> Wolfgang, is there some mailman setting that needs tweaking or looking >>>> at here? Thanks! >>>> >>> >>> Can this be a mailman issue? If Michal was CCed, is mailman involved in the >>> way to him? I would have thought that mail got delivered directly. >> >> I was thinking about the setting on if you get your own messages / ones >> you're on CC to or not, and if that was the copy say in Michal's inbox >> or U-Boot folder. > > Can you confirm how you received the message, directly or through the list? > > The TI e-mail team says the CCs were not removed prior to leaving TI. It looks like it went through list. M
Hi Peng, On 01/02/20 6:43 pm, Peng Fan wrote: >> Subject: Re: [PATCH v2 02/10] mmc: Add init() API >> >> Hi Simon, >> >> On 29/01/20 1:33 pm, Simon Goldschmidt wrote: >>> On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote: >>>> >>>> Add an init() API for platform specific init() operations. >>> >>> Could you describe why this cannot be done in the probe callback? It's >>> not easily visible as the function you changed (mmc_get_op_cond) >>> doesn't even have a comment to describe what it does... >> >> The reason is detailed in 06/10 patch description. probe() is always called for >> all MMC instances. I only want to switch on power (by calling >> sdhci_init()) and suffer the 1 second wait time when there is actually a card in >> the slot and user wants to access it. >>> >>> In general, I think commit messages could be more detailed than one >>> line. If only to make it easier in the future to recap why things have been >> done. >>> >> >> You're right. I will add a more detailed patch description in v2. > > This patch is v2. Please v3. > > Could a quirk be added in probe and if card not there, just bypass? > Or you want quick boot to avoid probe all the controllers? > The issue is that we need to wait for 1 second to detect the card itself. I want to move that delay out from probe(). BTW did you receive this mail directly or through the list? I had CC'd you but its not there in the patchwork headers. Thanks, Faiz
> Subject: Re: [PATCH v2 02/10] mmc: Add init() API > > Hi Peng, > > On 01/02/20 6:43 pm, Peng Fan wrote: > >> Subject: Re: [PATCH v2 02/10] mmc: Add init() API > >> > >> Hi Simon, > >> > >> On 29/01/20 1:33 pm, Simon Goldschmidt wrote: > >>> On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote: > >>>> > >>>> Add an init() API for platform specific init() operations. > >>> > >>> Could you describe why this cannot be done in the probe callback? > >>> It's not easily visible as the function you changed > >>> (mmc_get_op_cond) doesn't even have a comment to describe what it > does... > >> > >> The reason is detailed in 06/10 patch description. probe() is always > >> called for all MMC instances. I only want to switch on power (by > >> calling > >> sdhci_init()) and suffer the 1 second wait time when there is > >> actually a card in the slot and user wants to access it. > >>> > >>> In general, I think commit messages could be more detailed than one > >>> line. If only to make it easier in the future to recap why things > >>> have been > >> done. > >>> > >> > >> You're right. I will add a more detailed patch description in v2. > > > > This patch is v2. Please v3. > > > > Could a quirk be added in probe and if card not there, just bypass? > > Or you want quick boot to avoid probe all the controllers? > > > > The issue is that we need to wait for 1 second to detect the card itself. I want > to move that delay out from probe(). Understand. Then I am ok for patch. Please add more info in commit log in new version patchset. > > BTW did you receive this mail directly or through the list? I had CC'd you but > its not there in the patchwork headers. I am in the cc list of your first mail, but not from Simon's reply mail. Thanks, Peng. > > Thanks, > Faiz
Hi Tom, Wolfgang, On 03/02/20 5:34 pm, Peng Fan wrote: >> Subject: Re: [PATCH v2 02/10] mmc: Add init() API >> >> Hi Peng, >> >> On 01/02/20 6:43 pm, Peng Fan wrote: >>>> Subject: Re: [PATCH v2 02/10] mmc: Add init() API >>>> >>>> Hi Simon, >>>> >>>> On 29/01/20 1:33 pm, Simon Goldschmidt wrote: >>>>> On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas <faiz_abbas@ti.com> wrote: >>>>>> >>>>>> Add an init() API for platform specific init() operations. >>>>> >>>>> Could you describe why this cannot be done in the probe callback? >>>>> It's not easily visible as the function you changed >>>>> (mmc_get_op_cond) doesn't even have a comment to describe what it >> does... >>>> >>>> The reason is detailed in 06/10 patch description. probe() is always >>>> called for all MMC instances. I only want to switch on power (by >>>> calling >>>> sdhci_init()) and suffer the 1 second wait time when there is >>>> actually a card in the slot and user wants to access it. >>>>> >>>>> In general, I think commit messages could be more detailed than one >>>>> line. If only to make it easier in the future to recap why things >>>>> have been >>>> done. >>>>> >>>> >>>> You're right. I will add a more detailed patch description in v2. >>> >>> This patch is v2. Please v3. >>> >>> Could a quirk be added in probe and if card not there, just bypass? >>> Or you want quick boot to avoid probe all the controllers? >>> >> >> The issue is that we need to wait for 1 second to detect the card itself. I want >> to move that delay out from probe(). > > Understand. Then I am ok for patch. > > Please add more info in commit log in new version patchset. > >> >> BTW did you receive this mail directly or through the list? I had CC'd you but >> its not there in the patchwork headers. > > I am in the cc list of your first mail, but not from Simon's reply mail. > So Peng got the email but the list is dropping CCs after it gets them. How do I avoid this in the future? Should I always add maintainers in To? Thanks, Faiz
Dear Faiz, In message <04e0144c-cad3-d242-0393-ba33afa3dd7a@ti.com> you wrote: > > > I am in the cc list of your first mail, but not from Simon's reply mail. > > So Peng got the email but the list is dropping CCs after it gets them. > How do I avoid this in the future? Should I always add maintainers in To? We are investigating this. I _think_ (but this needs to be verified, I just had a short look) that Mailman might try to bee too clever; my speculation is that it might remove addresses from the Cc: list wich have the "nodupes" option set in their profile. Maybe mailman "thinks" that this is what it should do - otherwise the recipient would receive dupes at least for a reply-to-all, one trough the mailing list and the other through the Cc: But as mentioned, this needs to be investigated. I can see this ancient bug report [1], where a reply reads: In any case, this behavior is intentional by design. Cc: recipients who are list members with their 'avoid duplicates' option set are removed from the Cc: list to keep that list from growing excessively in long threads with many 'reply-all' replies. [1] https://bugs.launchpad.net/mailman/+bug/1216960 So apparently a solution/workaoround could be to globally remove the "nodupes" option for all subscribers, but I'm not sure if this is what we should do. I feel the key problem here is that we expect something from the mailing list that it has not been designed for - the differentiation between "this is just some random posting" and "this is a posting which is specifically addressed to you". this may or may not work, and it depends on several factors - how the mailing list tool works, and how the recipient filters his incoming e-mail. But I don't have any clever solution either. Best regards, Wolfgang Denk
On Sun, Feb 09, 2020 at 02:38:15PM +0100, Wolfgang Denk wrote: > Dear Faiz, > > In message <04e0144c-cad3-d242-0393-ba33afa3dd7a@ti.com> you wrote: > > > > > I am in the cc list of your first mail, but not from Simon's reply mail. > > > > So Peng got the email but the list is dropping CCs after it gets them. > > How do I avoid this in the future? Should I always add maintainers in To? > > We are investigating this. I _think_ (but this needs to be > verified, I just had a short look) that Mailman might try to bee too > clever; my speculation is that it might remove addresses from the > Cc: list wich have the "nodupes" option set in their profile. > Maybe mailman "thinks" that this is what it should do - otherwise > the recipient would receive dupes at least for a reply-to-all, one > trough the mailing list and the other through the Cc: > > But as mentioned, this needs to be investigated. > > I can see this ancient bug report [1], where a reply reads: > > In any case, this behavior is intentional by design. Cc: > recipients who are list members with their 'avoid duplicates' > option set are removed from the Cc: list to keep that list > from growing excessively in long threads with many > 'reply-all' replies. > > [1] https://bugs.launchpad.net/mailman/+bug/1216960 > > > So apparently a solution/workaoround could be to globally remove the > "nodupes" option for all subscribers, but I'm not sure if this is > what we should do. > > > I feel the key problem here is that we expect something from the > mailing list that it has not been designed for - the differentiation > between "this is just some random posting" and "this is a posting > which is specifically addressed to you". this may or may not work, > and it depends on several factors - how the mailing list tool works, > and how the recipient filters his incoming e-mail. > > But I don't have any clever solution either. I'm a little wary of changing the global setting for everyone as well. Perhaps we just need to note the problem happened for now and see if it really happens again in the future, such that we need to consider such a heavy-handed work-around. Thanks for looking into this more!
Dear Tom, In message <20200210142800.GJ13379@bill-the-cat> you wrote: > > I'm a little wary of changing the global setting for everyone as well. > Perhaps we just need to note the problem happened for now and see if it > really happens again in the future, such that we need to consider such a > heavy-handed work-around. Thanks for looking into this more! We will continue to look into this a bit deeper; I will report back. Best regards, Wolfgang Denk
Dear Tom, In message <20200211135633.8B4D02403FE@gemini.denx.de> I wrote: > > > I'm a little wary of changing the global setting for everyone as well. > > Perhaps we just need to note the problem happened for now and see if it > > really happens again in the future, such that we need to consider such a > > heavy-handed work-around. Thanks for looking into this more! > > We will continue to look into this a bit deeper; I will report back. Running some more tests confirms that 1) mailman indeed removes entries from the Cc: address list, if these refer to subscribers _and_ these have the "nodupes" flag set; 2) unsetting the "nodupes" option makes the problem go away. The statement in [1] suggests that this is considered "intentional by design", though I cannot follow the argument that otherwise Cc: lists would grow in long threads. Apparently there are many different opinions, see for example [3]. The code snippet given in the original bug description [2] is still about the same in the version of mailman which we are running (v2.1.26) so it should be easy to remove the responsible "else" branch... So an easy work around for the problem is to clear the "nodupes" setting in your subscription - alternatively we can try and patch mailman to behave like we want it. What do you think should we do? [1] https://bugs.launchpad.net/mailman/+bug/1216960/comments/1 [2] https://bugs.launchpad.net/mailman/+bug/1216960 [3] https://bugs.launchpad.net/mailman/+bug/266682 Best regards, Wolfgang Denk
Dear Tom, In message <20200211150800.D895B2403FE@gemini.denx.de> I wrote: > > So an easy work around for the problem is to clear the "nodupes" > setting in your subscription - alternatively we can try and patch > mailman to behave like we want it. There is even a clean approach upstream [1]: [1] https://bazaar.launchpad.net/~mailman-coders/mailman/2.1/revision/1825 Apparently the first RC where this commit is included is mailman 2.1.30rc1; we expect it will take ~ 1 month for v2.1.30 to get released, 2 months max. If it's OK we will jsut wait for this release, then upgrade, and configure with this option set: 1446 # The process which avoids sending a list copy of a message to a member who 1447 # is also directly addressed in To: or Cc: can drop the address from Cc: to 1448 # avoid growing a long Cc: list in long threads. This can be undesirable as 1449 # it can break DKIM signatures and possibly cause confusion. To avoid changes 1450 # to Cc: headers, set the list's drop_cc to No. 1451 DEFAULT_DROP_CC = Yes Agreed? Best regards, Wolfgang Denk
On Tue, Feb 11, 2020 at 05:25:39PM +0100, Wolfgang Denk wrote: > Dear Tom, > > In message <20200211150800.D895B2403FE@gemini.denx.de> I wrote: > > > > So an easy work around for the problem is to clear the "nodupes" > > setting in your subscription - alternatively we can try and patch > > mailman to behave like we want it. > > There is even a clean approach upstream [1]: > > > [1] https://bazaar.launchpad.net/~mailman-coders/mailman/2.1/revision/1825 > > Apparently the first RC where this commit is included is mailman > 2.1.30rc1; we expect it will take ~ 1 month for v2.1.30 to get > released, 2 months max. If it's OK we will jsut wait for this > release, then upgrade, and configure with this option set: > > 1446 # The process which avoids sending a list copy of a message to a member who > 1447 # is also directly addressed in To: or Cc: can drop the address from Cc: to > 1448 # avoid growing a long Cc: list in long threads. This can be undesirable as > 1449 # it can break DKIM signatures and possibly cause confusion. To avoid changes > 1450 # to Cc: headers, set the list's drop_cc to No. > 1451 DEFAULT_DROP_CC = Yes > > > Agreed? Agreed, thanks again!
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index d43983d4a6..50df8c8626 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -2787,14 +2787,13 @@ int mmc_get_op_cond(struct mmc *mmc) } if (err) return err; - #if CONFIG_IS_ENABLED(DM_MMC) - /* The device has already been probed ready for use */ -#else - /* made sure it's not NULL earlier */ - err = mmc->cfg->ops->init(mmc); - if (err) - return err; + struct dm_mmc_ops *ops = mmc_get_ops(mmc->dev); + if (ops->init) { + err = ops->init(mmc->dev); + if (err) + return err; + } #endif mmc->ddr_mode = 0; diff --git a/include/mmc.h b/include/mmc.h index 2f21dbf1b7..6aef125f25 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -406,6 +406,13 @@ struct mmc; #if CONFIG_IS_ENABLED(DM_MMC) struct dm_mmc_ops { + /** + * init() - platform specific initialization for the host controller + * + * @dev: Device to init + * @return 0 if Ok, -ve if error + */ + int (*init)(struct udevice *dev); /** * send_cmd() - Send a command to the MMC device *