Re: [PATCH v2 02/10] mmc: Add init() API
On Tue, Feb 11, 2020 at 05:25:39PM +0100, Wolfgang Denk wrote: > Dear Tom, > > In message <20200211150800.d895b240...@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! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 02/10] mmc: Add init() API
Dear Tom, In message <20200211150800.d895b240...@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 -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de "If a computer can't directly address all the RAM you can use, it's just a toy." - anonymous comp.sys.amiga posting, non-sequitir
Re: [PATCH v2 02/10] mmc: Add init() API
Dear Tom, In message <20200211135633.8b4d0240...@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 -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de "All my life I wanted to be someone; I guess I should have been more specific." - Jane Wagner
Re: [PATCH v2 02/10] mmc: Add init() API
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 -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Do not follow where the path may leadgo instead where there is no path and leave a trail.
Re: [PATCH v2 02/10] mmc: Add init() API
On Sun, Feb 09, 2020 at 02:38:15PM +0100, Wolfgang Denk wrote: > Dear Faiz, > > In message <04e0144c-cad3-d242-0393-ba33afa3d...@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! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 02/10] mmc: Add init() API
Dear Faiz, In message <04e0144c-cad3-d242-0393-ba33afa3d...@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 -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de A Sense of humor can help you overlook the unattractive, tolerate the unpleasant, cope with the unexpected and smile through the unbearable. - Moshe Waldoks
Re: [PATCH v2 02/10] mmc: Add init() API
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 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
RE: [PATCH v2 02/10] mmc: Add init() API
> 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 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
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 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
Re: [PATCH v2 02/10] mmc: Add init() API
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 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 >>>>>>>> To: >>>>>>>> CC: , , < >>>> lokeshvu...@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 (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
Re: [PATCH v2 02/10] mmc: Add init() API
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 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 >>>>>>> To: >>>>>>> CC: , , < >>> lokeshvu...@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 (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
RE: [PATCH v2 02/10] mmc: Add init() API
> 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 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
Re: [PATCH v2 02/10] mmc: Add init() API
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 -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de The Buddha, the Godhead, resides quite as comfortably in the circuits of a digital computer or the gears of a cycle transmission as he does at the top of a mountain or in the petals of a flower. - R. Pirsig, "Zen and the Art of Motorcycle Maintenance"
Re: [PATCH v2 02/10] mmc: Add init() API
Dear Simon, In message 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 -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de "355/113 -- Not the famous irrational number PI, but an incredible simulation!"
Re: [PATCH v2 02/10] mmc: Add init() API
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 To: Cc: michal.si...@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 -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Ill-chosen abstraction is particularly evident in the design of the ADA runtime system. The interface to the ADA runtime system is so opaque that it is impossible to model or predict its performance, making it effectively useless for real-time systems. - Marc D. Donner and David H. Jameson.
Re: [PATCH v2 02/10] mmc: Add init() API
On Thu, Jan 30, 2020 at 04:35:40PM +0100, Simon Goldschmidt wrote: > Tom Rini 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 > > > >>> To: > > > >>> CC: , , < > > lokeshvu...@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 (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. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 02/10] mmc: Add init() API
Tom Rini 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 > > >>> To: > > >>> CC: , , < > lokeshvu...@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 (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
Re: [PATCH v2 02/10] mmc: Add init() API
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 > >>> To: > >>> CC: , , , > >>> , > >>> 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 (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 signature.asc Description: PGP signature
Re: [PATCH v2 02/10] mmc: Add init() API
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 >>> To: >>> CC: , , , >>> , >>> 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 (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
Re: [PATCH v2 02/10] mmc: Add init() API
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 >> To: >> CC: , , , >> , >> 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 (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
Re: [PATCH v2 02/10] mmc: Add init() API
Faiz Abbas 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 > To: > CC: , , , > , > 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 >
Re: [PATCH v2 02/10] mmc: Add init() API
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 > To: > CC: , , , > , > 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 (maintainer:MMC) u-boot@lists.denx.de (open list) I see Peng there. Thanks, Michal
Re: [PATCH v2 02/10] mmc: Add init() API
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 To: CC: , , , , 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
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 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
Re: [PATCH v2 02/10] mmc: Add init() API
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
wrote:
>
> On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas 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
> > Signed-off-by: Lokesh Vutla
> > ---
> > 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
> >
Re: [PATCH v2 02/10] mmc: Add init() API
On Fri, Jan 24, 2020 at 12:52 PM Faiz Abbas 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
> Signed-off-by: Lokesh Vutla
> ---
> 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
>
[PATCH v2 02/10] mmc: Add init() API
Add an init() API for platform specific init() operations.
Signed-off-by: Faiz Abbas
Signed-off-by: Lokesh Vutla
---
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;
+ 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
