Re: [GIT PULL] On-demand device probing
On Thu, Oct 22, 2015 at 11:53:31AM -0700, Frank Rowand wrote: > On 10/22/2015 7:44 AM, Greg Kroah-Hartman wrote: > > > > > > On Thu, Oct 22, 2015 at 11:05:11AM +0200, Tomeu Vizoso wrote: > >> But that's moot currently because Greg believes that the time spent > >> probing devices at boot time could be reduced enough so that the order > >> in which devices are probed becomes irrelevant. IME that would have to > >> be under 200ms so that the user doesn't notice and that's unicorn-far > >> from any bootlog I have ever seen. > > > > But as no one has actually produced a bootlog, how do you know that? > > Where exactly is your time being spent? What driver is causing long > > delays? Why is the long-delay-drivers not being done in their own > > thread? And most importantly, why are you ignoring the work that people > > did back in 2008 to solve the issue on other hardware platforms? > > > >> Given that downstreams are already carrying as many hacks as they > >> could think of to speed total boot up, I think this is effectively > >> telling them to go away. > > > > No I'm not, I'm asking for real data, not hand-wavy-this-is-going-to > > solve-the-random-issue-i'm-having type patch by putting random calls in > > semi-random subsystems all over the kernel. > > > > And when I ask for real data, you respond with the fact that you aren't > > trying to speed up boot time here at all, so what am I supposed to think > > I also had the understanding that this patch series was about improving > boot time. But I was kindly corrected that the behavior change was > getting the panel displaying stuff at an earlier point in the boot sequence, > _not_ completing the entire boot faster. > > The claim for the current series, in patch 0 in v7 is: > >With this series I get the kernel to output to the panel in 0.5s, >instead of 2.8s. > > Just to get side-tracked, one other approach at ordering to reduce > deferrals reported a modest boot time reduction for four boards and a > very slight boot time increase for one other board.) The report of boot > times with that approach was in: > > http://article.gmane.org/gmane.linux.drivers.devicetree/133010 > > from Alexander Holler. > > I have not searched further to see if there is more data of boot time > reductions from any of the other attempts to change driver binding > order to move dependencies before use of a resource. But whether > there is a performance improvement or not, there continues to be > a stream of developers creatively impacting the binding order for > their specific driver(s) or board. So it seems that maybe there > is an underlying problem, or we don't have adequate documentation > explaining how to avoid a need to order bindings, or the > documentation exists and is not being read. > > I have been defaulting to the position that has been asserted by > the device tree maintainters, that probe deferrals work just fine > for at least the majority of cases (and is the message I have been > sharing in my conference presentations about device tree). But I > suspect that there is at least a small minority of cases that are not > well served by probe deferral. (Not to be read as an endorsement of > this specific patch series, just a generic observation.) I agree, there might be some small numbers that this is a problem for, and if so, great, show us the boot logs where things are taking up all of the time, and we can work on resolving those issues. But without hard numbers / details, this all is just random hand-waving, and I don't like making core kernel changes on that basis. And no one else should ever want us to do that either. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] On-demand device probing
On Thu, Oct 22, 2015 at 11:05:11AM +0200, Tomeu Vizoso wrote: > But that's moot currently because Greg believes that the time spent > probing devices at boot time could be reduced enough so that the order > in which devices are probed becomes irrelevant. IME that would have to > be under 200ms so that the user doesn't notice and that's unicorn-far > from any bootlog I have ever seen. But as no one has actually produced a bootlog, how do you know that? Where exactly is your time being spent? What driver is causing long delays? Why is the long-delay-drivers not being done in their own thread? And most importantly, why are you ignoring the work that people did back in 2008 to solve the issue on other hardware platforms? > Given that downstreams are already carrying as many hacks as they > could think of to speed total boot up, I think this is effectively > telling them to go away. No I'm not, I'm asking for real data, not hand-wavy-this-is-going-to solve-the-random-issue-i'm-having type patch by putting random calls in semi-random subsystems all over the kernel. And when I ask for real data, you respond with the fact that you aren't trying to speed up boot time here at all, so what am I supposed to think other than that you don't care enough to do the real work and are trying to hack the driver core up instead. > Sorry for the rant, No apologies needed, it's cathartic at times :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] On-demand device probing
On Thu, Oct 22, 2015 at 11:05:11AM +0200, Tomeu Vizoso wrote: > On 21 October 2015 at 23:50, Frank Rowand wrote: > > On 10/21/2015 2:12 PM, Rob Herring wrote: > >> On Wed, Oct 21, 2015 at 1:18 PM, Frank Rowand > >> wrote: > >>> On 10/21/2015 9:27 AM, Mark Brown wrote: > On Wed, Oct 21, 2015 at 08:59:51AM -0700, Frank Rowand wrote: > > On 10/19/2015 5:34 AM, Tomeu Vizoso wrote: > > >> To be clear, I was saying that this series should NOT affect total > >> boot times much. > > > I'm confused. If I understood correctly, improving boot time was > > the key justification for accepting this patch set. For example, > > from "[PATCH v7 0/20] On-demand device probing": > > > >I have a problem with the panel on my Tegra Chromebook taking longer > >than expected to be ready during boot (Stéphane Marchesin reported > > what > >is basically the same issue in [0]), and have looked into ordered > >probing as a better way of solving this than moving nodes around in > > the > >DT or playing with initcall levels and linking order. > > > >... > > > >With this series I get the kernel to output to the panel in 0.5s, > >instead of 2.8s. > > Overall boot time and time to get some individual built in component up > and running aren't the same thing - what this'll do is get things up > more in the link order of the leaf consumers rather than deferring those > leaf consumers when their dependencies aren't ready yet. > >>> > >>> Thanks! I read too much into what was being improved. > >>> > >>> So this patch series, which on other merits may be a good idea, is as > >>> a by product solving a specific ordering issue, moving successful panel > >>> initialization to an earlier point in the boot sequence, if I now > >>> understand more correctly. > >>> > >>> In that context, this seems like yet another ad hoc way of causing the > >>> probe order to change in a way to solves one specific issue? Could > >>> it just as likely move the boot order of some other driver on some > >>> other board later, to the detriment of somebody else? > >> > >> Time to display on is important for many products. Having the console > >> up as early as possible is another case. CAN bus is another. This is a > >> real problem that is not just bad drivers. > > > > Yes, I agree. > > > > What I am seeing is that there continues to be a need for the ability > > to explicitly order at least some driver initialization (at some > > granularity), despite the push back against explicit ordering that > > has been present in the past. > > The important point that I have struggled to explain is that right now > for downstreams to influence the order in which devices are probed, > they have to carry a substantial amount of patches that cannot be ever > upstreamed. This fiddling with initcall levels and link order means > changing files that are very frequently changing, increasing the > amount of work when rebasing and increasing the probability of > regressions after a rebase. > > This just adds up to other shortcomings of mainline and ends up with > the net result of vendors getting stuck with 3.4 kernels on SoCs that > start production in 2015. Another consequence is that vendors don't > have a chance to upstream their stuff even if they cared. The > overarching goal of the project I'm in is to reduce those shortcomings > that downstreams have to workaround, to facilitate their involvement > upstream. The init order of drivers has no influence at all on the ability for companies to have their individual drivers merged upstream, please don't be so dramatic about this. Worst case, a vendor keeps a single patch to drivers/Makefile in their tree that reorders things, yes it will get conflicts on every release, but really, it's trivial to maintain if they wish to keep doing this type of thing. Again, it is _not_ the reason that we are living with 2million+ lines of code in vendor kernels. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] On-demand device probing
On Sun, Oct 18, 2015 at 08:29:31PM +0100, Mark Brown wrote: > On Fri, Oct 16, 2015 at 11:57:50PM -0700, Greg Kroah-Hartman wrote: > > > I can't see adding calls like this all over the tree just to solve a > > bus-specific problem, you are adding of_* calls where they aren't > > needed, or wanted, at all. > > This isn't bus specific, I'm not sure what makes you say that? You are making it bus-specific by putting these calls all over the tree in different bus subsystems semi-randomly for all I can determine. > > What is the root-problem of your delay in device probing? I read your > > last patch series and I can't seem to figure out what the issue is that > > this is solving in any "better" way from the existing deferred probing. > > So, I don't actually have any platforms that are especially bothered by > this (at least not for my use cases) so there's a bit of educated > guessing going on here but there's two broad things I'm aware of. > > One is that regardless of the actual performance of the system when > deferred probe goes off it splats errors all over the console which > makes it look like something is going wrong even if everything is fine > in the end. If lots of deferred probing happens then the volume gets > big too. People find this distracting, noisy and ugly - it obscures > actual issues and trains people to ignore errors. I do think this is a > reasonable concern and that it's worth trying to mitigate against > deferral for this reason alone. We don't want to just ignore the errors > and not print anything either since if the resource doesn't appear the > user needs to know what is preventing the driver from instantiating so > they can try to fix it. This has come up many times, I have no objection to just turning that message into a debug message that can be dynamically enabled for those people wanting to debug their systems for boot time issues. Please send a patch to do so. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] On-demand device probing
On Sat, Oct 17, 2015 at 03:39:20PM -0400, Rob Clark wrote: > On Sat, Oct 17, 2015 at 2:59 PM, Greg Kroah-Hartman > wrote: > > On Sat, Oct 17, 2015 at 02:45:34PM -0400, Rob Clark wrote: > >> On Sat, Oct 17, 2015 at 2:27 PM, Greg Kroah-Hartman > >> wrote: > >> > On Sat, Oct 17, 2015 at 01:54:43PM -0400, Rob Clark wrote: > >> >> On Sat, Oct 17, 2015 at 12:56 PM, Greg Kroah-Hartman > >> >> wrote: > >> >> >> I'm guessing the time is a matter of probing and undoing the probes > >> >> >> rather than slow h/w. We could maybe improve things by making sure > >> >> >> drivers move what they defer on to the beginning of probe, but that > >> >> >> seems like a horrible, fragile hack. > >> >> > > >> >> > How can calling probe and failing cause 2 seconds? How many different > >> >> > probe calls are failing here? Again, a boot log graph would be great > >> >> > to > >> >> > see as it will show the root cause, not just guessing at this. > >> >> > >> >> > >> >> just fwiw, but when you have a driver that depends on several other > >> >> drivers (which in turn depend on other drivers and so on), the amount > >> >> of probe-defer we end up seeing is pretty comical. Yeah, there > >> >> probably is some room to optimize by juggling around order drivers do > >> >> things in probe. But that doesn't solve the fundamental problem with > >> >> the current state, about probe order having no clue about > >> >> dependencies.. > >> > > >> > I can imagine it is a lot of iterations, but how long does it really > >> > take? How many different devices are involved that it takes multiple > >> > loops in order to finally work out the correct order? Where is the time > >> > delays here, just calling probe() and having it instantly return > >> > shouldn't take all that long. > >> > >> offhand, I think the dependencies go at *least* three levels deep.. > >> I'd say, from memory, I see drm/msm taking at least 5 or 6 tries to > >> get all the way through requesting it's various different > >> regulators/clks/gpios. > > > > And how long does that really take? Numbers please :) > > > >> I hadn't really paid attention to how many > >> tries the drivers I depend on go through. (Of those, I take clks from > >> two different clk drivers (which have dependency on a 3rd clk driver), > >> and regulators and gpio's come from at least two places, which in turn > >> have dependencies on clks, etc.) I don't have really good hard > >> numbers handy (since my observations of this are w/ console over uart > >> which effects timings, and so I see it taking much longer than 2sec).. > >> but the 2sec figure that Tomeu mentioned seemed pretty plausible to > >> me. > >> > >> I can try to get better #'s... I should have my kernel hat on at least > >> some of the time next week.. but the 2sec figure didn't seem > >> unrealistic to me. > > > > Based on the time it takes a modern laptop to boot, 2 seconds is > > forever, there has to be something else going on here other than just > > calling probe() a bunch of times. Please use the tools we have to > > determine this before trying to change the driver core. > > yes, I am aware of the tools.. although so far I spend most of my time > just trying to get things working in the first place ;-) And that's where most people stop, if you want to make it fast, you have to put in more effort, sorry. Don't expect the driver core to work around driver bugs for you. > All I was trying to point out was that Tomeu's figures didn't really > seem unrealistic. I mean, given that the average SoC driver probably > depends on at least one clock and at least one regulator, having to > probe each driver at least twice seems plausible. And that having a > noticeable effect on boot time doesn't seem surprising. I'm not sure > that saying 'modern laptop can boot in 2sec' adds much to the > discussion since I don't think you have quite so much interdependency > between devices vs random probe order. I have seen arm devices boot > to UI in similar times, but that was pre-devicetree days. 2 extra probes add a second to the boot time? Those sound like really broken drivers to me :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] On-demand device probing
On Sat, Oct 17, 2015 at 02:45:34PM -0400, Rob Clark wrote: > On Sat, Oct 17, 2015 at 2:27 PM, Greg Kroah-Hartman > wrote: > > On Sat, Oct 17, 2015 at 01:54:43PM -0400, Rob Clark wrote: > >> On Sat, Oct 17, 2015 at 12:56 PM, Greg Kroah-Hartman > >> wrote: > >> >> I'm guessing the time is a matter of probing and undoing the probes > >> >> rather than slow h/w. We could maybe improve things by making sure > >> >> drivers move what they defer on to the beginning of probe, but that > >> >> seems like a horrible, fragile hack. > >> > > >> > How can calling probe and failing cause 2 seconds? How many different > >> > probe calls are failing here? Again, a boot log graph would be great to > >> > see as it will show the root cause, not just guessing at this. > >> > >> > >> just fwiw, but when you have a driver that depends on several other > >> drivers (which in turn depend on other drivers and so on), the amount > >> of probe-defer we end up seeing is pretty comical. Yeah, there > >> probably is some room to optimize by juggling around order drivers do > >> things in probe. But that doesn't solve the fundamental problem with > >> the current state, about probe order having no clue about > >> dependencies.. > > > > I can imagine it is a lot of iterations, but how long does it really > > take? How many different devices are involved that it takes multiple > > loops in order to finally work out the correct order? Where is the time > > delays here, just calling probe() and having it instantly return > > shouldn't take all that long. > > offhand, I think the dependencies go at *least* three levels deep.. > I'd say, from memory, I see drm/msm taking at least 5 or 6 tries to > get all the way through requesting it's various different > regulators/clks/gpios. And how long does that really take? Numbers please :) > I hadn't really paid attention to how many > tries the drivers I depend on go through. (Of those, I take clks from > two different clk drivers (which have dependency on a 3rd clk driver), > and regulators and gpio's come from at least two places, which in turn > have dependencies on clks, etc.) I don't have really good hard > numbers handy (since my observations of this are w/ console over uart > which effects timings, and so I see it taking much longer than 2sec).. > but the 2sec figure that Tomeu mentioned seemed pretty plausible to > me. > > I can try to get better #'s... I should have my kernel hat on at least > some of the time next week.. but the 2sec figure didn't seem > unrealistic to me. Based on the time it takes a modern laptop to boot, 2 seconds is forever, there has to be something else going on here other than just calling probe() a bunch of times. Please use the tools we have to determine this before trying to change the driver core. > Just as an aside, the amount of probe-defer adds quite a lot of noise > when you are trying to debug why some driver doesn't probe > successfully. Which itself would be a nice reason to do something > more clever.. People seem to not like the noise, so let's turn off those messages, that should speed things up :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] On-demand device probing
On Sat, Oct 17, 2015 at 01:54:43PM -0400, Rob Clark wrote: > On Sat, Oct 17, 2015 at 12:56 PM, Greg Kroah-Hartman > wrote: > >> I'm guessing the time is a matter of probing and undoing the probes > >> rather than slow h/w. We could maybe improve things by making sure > >> drivers move what they defer on to the beginning of probe, but that > >> seems like a horrible, fragile hack. > > > > How can calling probe and failing cause 2 seconds? How many different > > probe calls are failing here? Again, a boot log graph would be great to > > see as it will show the root cause, not just guessing at this. > > > just fwiw, but when you have a driver that depends on several other > drivers (which in turn depend on other drivers and so on), the amount > of probe-defer we end up seeing is pretty comical. Yeah, there > probably is some room to optimize by juggling around order drivers do > things in probe. But that doesn't solve the fundamental problem with > the current state, about probe order having no clue about > dependencies.. I can imagine it is a lot of iterations, but how long does it really take? How many different devices are involved that it takes multiple loops in order to finally work out the correct order? Where is the time delays here, just calling probe() and having it instantly return shouldn't take all that long. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] On-demand device probing
On Sat, Oct 17, 2015 at 11:28:29AM -0500, Rob Herring wrote: > On Sat, Oct 17, 2015 at 10:47 AM, Greg Kroah-Hartman > wrote: > > On Sat, Oct 17, 2015 at 10:04:55AM -0500, Rob Herring wrote: > >> On Sat, Oct 17, 2015 at 1:57 AM, Greg Kroah-Hartman > >> wrote: > >> > On Wed, Oct 14, 2015 at 10:34:00AM +0200, Tomeu Vizoso wrote: > >> >> Hi Rob, > >> >> > >> >> here is the pull request you asked for, with no changes from the version > >> >> that I posted last to the list. > >> >> > >> >> The following changes since commit > >> >> 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f: > >> >> > >> >> Linux 4.3-rc1 (2015-09-12 16:35:56 -0700) > >> >> > >> >> are available in the git repository at: > >> >> > >> >> git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git > >> >> on-demand-probes-for-next > >> > > >> > That's not a signed tag :( > >> > > >> > Anyway, I REALLY don't like this series (sorry for the delay in > >> > reviewing them, normally I trust Rob's judgement...) > >> > >> We've seen a lot of attempts here. This is really the best solution so > >> far in that it is simple, uses existing data from DT, and was low risk > >> for breaking platforms (at least I thought it would be). Anyway, > >> getting more exposure is why I've put it into -next. > > > > Exposure is good, now we know it breaks some builds, which was useful :) > > Now that I've looked at them, they are somewhat questionable failures. > They do show the fragile nature of probe ordering and the implicit > dependencies we have. > > >> > I can't see adding calls like this all over the tree just to solve a > >> > bus-specific problem, you are adding of_* calls where they aren't > >> > needed, or wanted, at all. > >> > >> I think Linus W, Mark B, and I all said a similar thing initially in > >> that dependencies should be handled in the driver core. We went down > >> the path of making this not firmware (aka bus) specific and an earlier > >> version had just that (with fwnode_* calls). That turned out to be > >> pointless as the calling locations were almost always in DT specific > >> code anyway. If you notice, the calls are next to other DT specific > >> calls generally (usually a "get"). So yes, I'd prefer not to have to > >> touch every subsystem, but we had to do that anyway to add DT support. > > > > If they are "next" to a call like that, why not put it in that call? I > > really object to having to "sprinkle" this all over the kernel, for no > > obvious reason why that is happening at all (look at the USB patch for > > one such example.) > > Looking at it again, they are in DT specific code already. The USB one > is in devm_usb_get_phy_by_node() which is a DT specific call. But that's not very obvious, right? Especially given that you now have to add a new .h file, which implies that suddenly this file is now touching a new subsystem. > >> We've generally split the DT code into the core (in drivers/of) and > >> the binding specific (in subsystems). Extracting dependency > >> information the DT is going to require binding specific knowledge, so > >> subsystem changes are probably unavoidable. > >> > >> The alternative is we put binding specific knowledge into the core DT > >> code to parse dependencies. > >> > >> > What is the root-problem of your delay in device probing? I read your > >> > last patch series and I can't seem to figure out what the issue is that > >> > this is solving in any "better" way from the existing deferred probing. > >> > >> It saves 2 seconds in the boot time as re-probing takes time. That > >> alone seems compelling to me. > > > > 2 seconds is _forever_, and really seems like some other driver is > > sleeping and causing this problem. What does the bootlog time-chart say > > is really causing this long delay? There's no way we are stuck in some > > sort of logic loop for that long (i.e. having to walk the list of > > devices somehow.) This sounds like a driver-specific problem that is > > being worked around by having to touch all subsystems, which isn't nice. > > I don't think it is one driver as the improvement is seen on multiple > platforms. I'll let Tomeu comment furthe
Re: [GIT PULL] On-demand device probing
On Sat, Oct 17, 2015 at 10:04:55AM -0500, Rob Herring wrote: > On Sat, Oct 17, 2015 at 1:57 AM, Greg Kroah-Hartman > wrote: > > On Wed, Oct 14, 2015 at 10:34:00AM +0200, Tomeu Vizoso wrote: > >> Hi Rob, > >> > >> here is the pull request you asked for, with no changes from the version > >> that I posted last to the list. > >> > >> The following changes since commit > >> 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f: > >> > >> Linux 4.3-rc1 (2015-09-12 16:35:56 -0700) > >> > >> are available in the git repository at: > >> > >> git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git > >> on-demand-probes-for-next > > > > That's not a signed tag :( > > > > Anyway, I REALLY don't like this series (sorry for the delay in > > reviewing them, normally I trust Rob's judgement...) > > We've seen a lot of attempts here. This is really the best solution so > far in that it is simple, uses existing data from DT, and was low risk > for breaking platforms (at least I thought it would be). Anyway, > getting more exposure is why I've put it into -next. Exposure is good, now we know it breaks some builds, which was useful :) > > I can't see adding calls like this all over the tree just to solve a > > bus-specific problem, you are adding of_* calls where they aren't > > needed, or wanted, at all. > > I think Linus W, Mark B, and I all said a similar thing initially in > that dependencies should be handled in the driver core. We went down > the path of making this not firmware (aka bus) specific and an earlier > version had just that (with fwnode_* calls). That turned out to be > pointless as the calling locations were almost always in DT specific > code anyway. If you notice, the calls are next to other DT specific > calls generally (usually a "get"). So yes, I'd prefer not to have to > touch every subsystem, but we had to do that anyway to add DT support. If they are "next" to a call like that, why not put it in that call? I really object to having to "sprinkle" this all over the kernel, for no obvious reason why that is happening at all (look at the USB patch for one such example.) > We've generally split the DT code into the core (in drivers/of) and > the binding specific (in subsystems). Extracting dependency > information the DT is going to require binding specific knowledge, so > subsystem changes are probably unavoidable. > > The alternative is we put binding specific knowledge into the core DT > code to parse dependencies. > > > What is the root-problem of your delay in device probing? I read your > > last patch series and I can't seem to figure out what the issue is that > > this is solving in any "better" way from the existing deferred probing. > > It saves 2 seconds in the boot time as re-probing takes time. That > alone seems compelling to me. 2 seconds is _forever_, and really seems like some other driver is sleeping and causing this problem. What does the bootlog time-chart say is really causing this long delay? There's no way we are stuck in some sort of logic loop for that long (i.e. having to walk the list of devices somehow.) This sounds like a driver-specific problem that is being worked around by having to touch all subsystems, which isn't nice. Hint, we didn't have to do this type of thing to solve boot delays on x86 when we had hardware that was slow to initialize, why should DT be special? :) > Another downside to deferred probing is you have to touch every driver > and subsystem to support it. This contains the problem to the > subsystems. But we have deferred probing already, only those drivers that need/want it have to do anything, why create yet-another model here? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] On-demand device probing
On Wed, Oct 14, 2015 at 10:34:00AM +0200, Tomeu Vizoso wrote: > Hi Rob, > > here is the pull request you asked for, with no changes from the version > that I posted last to the list. > > The following changes since commit 6ff33f3902c3b1c5d0db6b1e2c70b6d76fba357f: > > Linux 4.3-rc1 (2015-09-12 16:35:56 -0700) > > are available in the git repository at: > > git+ssh://git.collabora.co.uk/git/user/tomeu/linux.git > on-demand-probes-for-next That's not a signed tag :( Anyway, I REALLY don't like this series (sorry for the delay in reviewing them, normally I trust Rob's judgement...) I can't see adding calls like this all over the tree just to solve a bus-specific problem, you are adding of_* calls where they aren't needed, or wanted, at all. What is the root-problem of your delay in device probing? I read your last patch series and I can't seem to figure out what the issue is that this is solving in any "better" way from the existing deferred probing. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] use devicemodel with parport
On Mon, Jun 01, 2015 at 11:16:51AM +0530, Sudip Mukherjee wrote: > On Mon, Jun 01, 2015 at 07:05:30AM +0900, Greg Kroah-Hartman wrote: > > On Wed, May 20, 2015 at 08:56:56PM +0530, Sudip Mukherjee wrote: > > > After 5 versions of WIP, finally a patch submission. > > > parport subsystem is now in the transition stage and supports the old > > > model and the new device model. 3 of the drivers have been converted > > > into new model and tested. > > > After other drivers are converted we can remove the old code from > > > parport. > > > > This looks good, very nice job. > Thanks, but its not me alone. Dan, Jean, Alan has helped me a lot in this. > > > > Are you going to continue to fix up the other drivers? I don't want us > > to have a half-converted subsystem in the kernel. > ofcourse yes. I also donot want to be a maintainer of half converted > subsystem. :) > i am thinking, after it is in the linux-next for a few days and seeing > any effects of these changes I will start with the other drivers. > and after all drivers are converted and that is also in next for sufficient > days I will start with the cleanup of the old codes. Sounds like a good plan. > And then there are some pending bugs in bugzilla. I am also planning to > add the code for epst in paride (i saw in many forums that people are > not able to use their scanner on latest kernels). "epst"? What's that? -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] use devicemodel with parport
On Wed, May 20, 2015 at 08:56:56PM +0530, Sudip Mukherjee wrote: > After 5 versions of WIP, finally a patch submission. > parport subsystem is now in the transition stage and supports the old > model and the new device model. 3 of the drivers have been converted > into new model and tested. > After other drivers are converted we can remove the old code from > parport. This looks good, very nice job. Are you going to continue to fix up the other drivers? I don't want us to have a half-converted subsystem in the kernel. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] MAINTAINERS: maintain parport
On Thu, May 21, 2015 at 07:51:18AM +0200, Willy Tarreau wrote: > On Wed, May 20, 2015 at 05:46:44PM +0200, Richard Weinberger wrote: > > On Wed, May 20, 2015 at 5:27 PM, Sudip Mukherjee > > wrote: > > > Lets give the parport subsystem a proper name and start > > > maintaining the files. > > > > Excuse me, but usually someone takes over the maintainer role after > > proving that he > > cares for a sub system for a certain period of time. > > Or did I miss something? > > Sudip has been volunteering for fixing this code which is marked > as "orphan". So whatever his experience and care in this area, > it will be better for the driver to have a maintainer than none. > > Good luck Sudip, as drivers which have become orphans are probably > the ones that deserve the least love :-) I agree. I've been "baby-sitting" this subsystem for a long time now. If Sudip wants to take it on, and based on his recent patches it looks like he does, I'm all for him to be the maintainer of it. So this patch in the series is fine with me, I'll apply it when the rest of this series gets merged. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] parport: modify parport subsystem to use devicemodel
On Wed, Apr 15, 2015 at 01:18:41PM +0530, Sudip Mukherjee wrote:
> @@ -29,6 +31,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -100,6 +103,11 @@ static struct parport_operations dead_ops = {
> .owner = NULL,
> };
>
> +struct bus_type parport_bus_type = {
> + .name = "parport",
> +};
> +EXPORT_SYMBOL(parport_bus_type);
They bus type shouldn't need to be exported, unless something is really
wrong. Why did you do this?
> +
> /* Call attach(port) for each registered driver. */
> static void attach_driver_chain(struct parport *port)
> {
> @@ -157,6 +165,7 @@ int parport_register_driver (struct parport_driver *drv)
>
> if (list_empty(&portlist))
> get_lowlevel_driver ();
> + drv->devmodel = false;
>
> mutex_lock(®istration_lock);
> list_for_each_entry(port, &portlist, list)
> @@ -167,6 +176,57 @@ int parport_register_driver (struct parport_driver *drv)
> return 0;
> }
>
> +/*
> + * __parport_register_drv - register a new parport driver
> + * @drv: the driver structure to register
> + * @owner: owner module of drv
> + * @mod_name: module name string
> + *
> + * Adds the driver structure to the list of registered drivers.
> + * Returns a negative value on error, otherwise 0.
> + * If no error occurred, the driver remains registered even if
> + * no device was claimed during registration.
> + */
> +int __parport_register_drv(struct parport_driver *drv,
> +struct module *owner, const char *mod_name)
> +{
> + struct parport *port;
> + int ret, err = 0;
> + bool attached = false;
You shouldn't need to track attached/not attached with the driver model,
that's what it is there for to handle for you.
> +
> + if (list_empty(&portlist))
> + get_lowlevel_driver();
what does this call do?
> +
> + /* initialize common driver fields */
> + drv->driver.name = drv->name;
> + drv->driver.bus = &parport_bus_type;
> + drv->driver.owner = owner;
> + drv->driver.mod_name = mod_name;
> + drv->devmodel = true;
> + ret = driver_register(&drv->driver);
> + if (ret)
> + return ret;
> +
> + mutex_lock(®istration_lock);
> + list_for_each_entry(port, &portlist, list) {
> + ret = drv->attach_ret(port, drv);
> + if (ret == 0)
> + attached = true;
> + else
> + err = ret;
> + }
> + if (attached)
> + list_add(&drv->list, &drivers);
Why all of this? You shouldn't need your own matching anymore, the
driver core does it for you when you register the driver, against the
devices you have already registered, if any.
> + mutex_unlock(®istration_lock);
> + if (!attached) {
> + driver_unregister(&drv->driver);
> + return err;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(__parport_register_drv);
> +
> /**
> * parport_unregister_driver - deregister a parallel port device driver
> * @drv: structure describing the driver that was given to
> @@ -193,11 +253,15 @@ void parport_unregister_driver (struct parport_driver
> *drv)
> list_for_each_entry(port, &portlist, list)
> drv->detach(port);
> mutex_unlock(®istration_lock);
> + if (drv->devmodel)
> + driver_unregister(&drv->driver);
> }
>
> -static void free_port (struct parport *port)
> +static void free_port(struct device *dev)
> {
> int d;
> + struct parport *port = to_parport_dev(dev);
> +
> spin_lock(&full_list_lock);
> list_del(&port->full_list);
> spin_unlock(&full_list_lock);
> @@ -223,8 +287,9 @@ static void free_port (struct parport *port)
>
> struct parport *parport_get_port (struct parport *port)
> {
> - atomic_inc (&port->ref_count);
> - return port;
> + struct device *dev = get_device(&port->ddev);
> +
> + return to_parport_dev(dev);
> }
>
> /**
> @@ -237,11 +302,7 @@ struct parport *parport_get_port (struct parport *port)
>
> void parport_put_port (struct parport *port)
> {
> - if (atomic_dec_and_test (&port->ref_count))
> - /* Can destroy it now. */
> - free_port (port);
> -
> - return;
> + put_device(&port->ddev);
> }
>
> /**
> @@ -281,6 +342,7 @@ struct parport *parport_register_port(unsigned long base,
> int irq, int dma,
> int num;
> int device;
> char *name;
> + int ret;
>
> tmp = kzalloc(sizeof(struct parport), GFP_KERNEL);
> if (!tmp) {
> @@ -333,6 +395,9 @@ struct parport *parport_register_port(unsigned long base,
> int irq, int dma,
>*/
> sprintf(name, "parport%d", tmp->portnum = tmp->number);
> tmp->name = name;
> + tmp->ddev.bus = &parport_bus_type;
> + tmp->ddev.release = free_port;
> + dev_set_name(&tmp->ddev, name);
>
> for (device = 0; device < 5; device++)
> /* assume
Re: [PATCH 1/4] parport: modify parport subsystem to use devicemodel
On Wed, Apr 15, 2015 at 01:18:41PM +0530, Sudip Mukherjee wrote: > --- a/drivers/parport/share.c > +++ b/drivers/parport/share.c > @@ -10,6 +10,8 @@ > * based on work by Grant Guenther > * and Philip Blundell > * > + * Added Device-Model - Sudip Mukherjee Changelog handles this, no need to add it to the file. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] parport: modify parport subsystem to use devicemodel
On Wed, Apr 15, 2015 at 01:18:41PM +0530, Sudip Mukherjee wrote:
> parport starts using device-model and we now have parport under
> /sys/bus. As the ports are discovered they are added as device under
> /sys/bus/parport. As and when other drivers register new device,
> they will be registered as a subdevice under the relevant parport.
>
> Signed-off-by: Sudip Mukherjee
> ---
> drivers/parport/procfs.c | 15 ++-
> drivers/parport/share.c | 236
> ---
> include/linux/parport.h | 29 +-
> 3 files changed, 267 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/parport/procfs.c b/drivers/parport/procfs.c
> index 3b47080..1ce363b 100644
> --- a/drivers/parport/procfs.c
> +++ b/drivers/parport/procfs.c
> @@ -21,6 +21,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -558,8 +559,18 @@ int parport_device_proc_unregister(struct pardevice
> *device)
>
> static int __init parport_default_proc_register(void)
> {
> + int ret;
> +
> parport_default_sysctl_table.sysctl_header =
> register_sysctl_table(parport_default_sysctl_table.dev_dir);
> + if (!parport_default_sysctl_table.sysctl_header)
> + return -ENOMEM;
> + ret = bus_register(&parport_bus_type);
> + if (ret) {
> + unregister_sysctl_table(parport_default_sysctl_table.
> + sysctl_header);
> + return ret;
> + }
> return 0;
> }
>
> @@ -570,6 +581,7 @@ static void __exit parport_default_proc_unregister(void)
> sysctl_header);
> parport_default_sysctl_table.sysctl_header = NULL;
> }
> + bus_unregister(&parport_bus_type);
> }
>
> #else /* no sysctl or no procfs*/
> @@ -596,11 +608,12 @@ int parport_device_proc_unregister(struct pardevice
> *device)
>
> static int __init parport_default_proc_register (void)
> {
> - return 0;
> + return bus_register(&parport_bus_type);
> }
>
> static void __exit parport_default_proc_unregister (void)
> {
> + bus_unregister(&parport_bus_type);
> }
> #endif
Don't bury bus register/unregister down in proc file creation, it's hard
to review and justify.
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] sc16is7xx: expose RTS inversion in RS-485 mode
On Tue, Mar 17, 2015 at 03:57:00PM +0100, Jakub Kiciński wrote: > On Tue, 17 Mar 2015 10:45:26 -0400 (EDT), Jon Ringle wrote: > > This makes sense. I did have to fix up my user space app to set > > SER_RS485_RTS_ON_SEND after applying this patch. > > Yes, perhaps I should have mentioned that this makes the ioctl return > -EINVAL when neither of SER_RS485_RTS_*_SEND flags is set. This > definitely has potential to break people's apps. Let's see if anyone > else has an opinion on this. Yes, don't break people's apps. I can't take this, sorry. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] i2c: Don't wait for device release in i2c_del_adapter
On Thu, Jan 15, 2015 at 06:25:22PM +0200, Pantelis Antoniou wrote: > Hi Greg, > > > On Jan 14, 2015, at 22:41 , Greg Kroah-Hartman > > wrote: > > > > On Wed, Jan 14, 2015 at 07:24:22PM +0200, Pantelis Antoniou wrote: > >> I’ll try to dig around tomorrow and see what the real device reference > >> counts > >> are, but my hunch goes like this: > >> > >> MUX > >> +—- ADAPTER > >>+— DEV. > >> > >> Mux remove method is called, i2c_del_mux_adapter is called on all the > >> channels > >> of the mux, calling in turn i2c_del_adapter which hangs on completion of > >> the > >> dev_released. > >> > >> The call to device_unregister never calls the device_type callback > >> (i2c_adapter_dev_release) > >> because the reference count is not 1 at that point, someone else is having > >> another > >> reference. > > > > First of all, my head hurts. Tracking device references ain’t easy. Is there > some kind > of debugging method you’d recommend for this? You can turn on debugging for kobjects and the driver core if you want to slow down your system log a bunch, but it can be helpful for trickier issues. Or just sprinkle a few printks around. > > I don't remember the i2c core code at all, that was a long time ago. > > > > Bummer. Do you remember code you wrote 12 year ago and haven't looked at for at least 11? Why expect others to as well? :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RFC] i2c: Don't wait for device release in i2c_del_adapter
On Wed, Jan 14, 2015 at 07:24:22PM +0200, Pantelis Antoniou wrote: > I’ll try to dig around tomorrow and see what the real device reference counts > are, but my hunch goes like this: > > MUX > +—- ADAPTER > +— DEV. > > Mux remove method is called, i2c_del_mux_adapter is called on all the channels > of the mux, calling in turn i2c_del_adapter which hangs on completion of the > dev_released. > > The call to device_unregister never calls the device_type callback > (i2c_adapter_dev_release) > because the reference count is not 1 at that point, someone else is having > another > reference. Who has that reference? It's not sysfs, right? Or is it? How is the remove method being called, is that coming from a sysfs file? Having that call "wait" for the other release call to happen is really old, as Jean points out, from 2003. We have "fixed" sysfs since then to detach the files from the devices easier, we used to have some nasy reference count issues in that area. Perhaps this isn't needed anymore, I don't remember the i2c core code at all, that was a long time ago. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] Driver for talking to PLX PEX8xxx PCIe switch over I2C
On Tue, Sep 30, 2014 at 02:35:51PM -0700, Guenter Roeck wrote: > On Tue, Sep 30, 2014 at 11:13:39PM +0200, Wolfram Sang wrote: > > > > > > Since this is a driver for a PCIe switch, I currently put it under > > > > /driver/pci/. > > > > I'm very open to suggestions for moving this around (Does introducing > > > > drivers/pci/switch/ seem any better?). Especially I can see that the > > > > drivers/pci/Kconfig appears under "Bus Options" in the main menu which > > > > does not > > > > look like the right place for this driver. I am looking for suggestions > > > > here. > > > > > > Definitely interesting work. I have no idea where to put it. It > > > doesn't have anything to do with PCI core services or structures, so > > > drivers/pci doesn't seem like the ideal place for it, but I don't have > > > any better ideas. If it did end up under drivers/pci, I agree that a > > > drivers/pci/switch or something similar would probably be better than > > > having it directly in drivers/pci. > > > > > > Maybe Wolfram or other I2C folks will have an idea. > > > > Hmm, is this a common interface for those switches? Then a directory > > like drivers/pci/switch could make sense. Otherwise I'd suggest > > drivers/misc? > > > We had discussed drivers/misc internally, but it didn't seem to be the right > location to me. After all, it is not a misc driver, but a driver to configure > a PCIe switch. Not that I have a better idea, though. drivers/misc is fine with me, I think we already have something like this for I2C USB devices somewhere in the tree in that location. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: Fix modalias for ACPI enumerated I2C devices
On Wed, Oct 16, 2013 at 01:04:02AM +0100, Grant Likely wrote:
> On Wed, Oct 16, 2013 at 12:47 AM, Rafael J. Wysocki wrote:
> > On Tuesday, October 15, 2013 04:31:43 PM Greg Kroah-Hartman wrote:
> >> On Tue, Oct 15, 2013 at 11:24:01PM +0200, Rafael J. Wysocki wrote:
> >> > On Tuesday, October 15, 2013 01:48:29 PM Greg Kroah-Hartman wrote:
> >> > > On Tue, Oct 15, 2013 at 10:37:02PM +0200, Rafael J. Wysocki wrote:
> >> > > > On Tuesday, October 15, 2013 07:44:44 PM Zhang Rui wrote:
> >> > > > > On Mon, 2013-10-14 at 20:47 +0800, Zhang Rui wrote:
> >> > > > > > On Mon, 2013-10-14 at 14:18 +0300, Jarkko Nikula wrote:
> >> > > > > > > On 10/14/2013 12:23 PM, Zhang Rui wrote:
> >> > > > > > > > Hi,
> >> > > > > > > >
> >> > > > > > > > On Thu, 2013-10-10 at 17:17 +0300, Jarkko Nikula wrote:
> >> > > > > > > >> There is a minor fault about ACPI enumerated I2C devices
> >> > > > > > > >> with their modalias
> >> > > > > > > >> attribute. Now modalias is set by device instance not by
> >> > > > > > > >> hardware ID.
> >> > > > > > > >> For example "i2c:INTABCD:00", "i2c:INTABCD:01" etc.
> >> > > > > > > >>
> >> > > > > > > >> This means each device instance gets different modalias
> >> > > > > > > >> which does match
> >> > > > > > > >> with generated modules.alias. Currently this is not problem
> >> > > > > > > >> as matching can
> >> > > > > > > >> happen also with "acpi:INTABCD" modalias.
> >> > > > > > > >>
> >> > > > > > > > IMO, this is not the proper fix for the modalias problem
> >> > > > > > > > because ACPI
> >> > > > > > > > enumerated I2C device may have compatible ids.
> >> > > > > > > > Instead, we should export all the compatible ids as the
> >> > > > > > > > modules alias of
> >> > > > > > > > the ACPI enumerated I2C device.
> >> > > > > > > >
> >> > > > > > > > can you please take a look at the patch I sent out earlier?
> >> > > > > > > > https://patchwork.kernel.org/patch/3034991/
> >> > > > > > > > https://patchwork.kernel.org/patch/3035041/
> >> > > > > > > > https://patchwork.kernel.org/patch/3035021/
> >> > > > > > > I see. This makes sense as it avoids that same device has two
> >> > > > > > > different
> >> > > > > > > modaliases from both acpi and other subsystem.
> >> > > > > > >
> >> > > > > > > How about modalias nodes in sysfs, should they also reflect
> >> > > > > > > what is
> >> > > > > > > matching uvent?
> >> > > > > > >
> >> > > > > > good catch, will fix "modalias" as well in next version.
> >> > > > >
> >> > > > > Hi,
> >> > > > >
> >> > > > > I have a question about the device "uevent" and "modalias" sysfs
> >> > > > > attributes.
> >> > > > > what is the relationship between these two?
> >> > > > > Am I right to say that, if there is the "MODALIAS" field in uevent
> >> > > > > file,
> >> > > > > this field must be consistent with the content in "modalias"
> >> > > > > attribute?
> >> > >
> >> > > Well, if it isn't, it's pretty pointless, right?
> >> > >
> >> > > > > I checked the code in drivers/base/platform.c,
> >> > > > > static ssize_t modalias_show(struct device *dev, struct
> >> > > > > device_attribute
> >> > > > > *a,
> >> > > > > char *buf)
> >> > > > > {
> >> > > > > struct platform_device *pdev = to_platform_device(dev);
> >> > > > > i
Re: [PATCH] i2c: Fix modalias for ACPI enumerated I2C devices
On Tue, Oct 15, 2013 at 11:24:01PM +0200, Rafael J. Wysocki wrote:
> On Tuesday, October 15, 2013 01:48:29 PM Greg Kroah-Hartman wrote:
> > On Tue, Oct 15, 2013 at 10:37:02PM +0200, Rafael J. Wysocki wrote:
> > > On Tuesday, October 15, 2013 07:44:44 PM Zhang Rui wrote:
> > > > On Mon, 2013-10-14 at 20:47 +0800, Zhang Rui wrote:
> > > > > On Mon, 2013-10-14 at 14:18 +0300, Jarkko Nikula wrote:
> > > > > > On 10/14/2013 12:23 PM, Zhang Rui wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > On Thu, 2013-10-10 at 17:17 +0300, Jarkko Nikula wrote:
> > > > > > >> There is a minor fault about ACPI enumerated I2C devices with
> > > > > > >> their modalias
> > > > > > >> attribute. Now modalias is set by device instance not by
> > > > > > >> hardware ID.
> > > > > > >> For example "i2c:INTABCD:00", "i2c:INTABCD:01" etc.
> > > > > > >>
> > > > > > >> This means each device instance gets different modalias which
> > > > > > >> does match
> > > > > > >> with generated modules.alias. Currently this is not problem as
> > > > > > >> matching can
> > > > > > >> happen also with "acpi:INTABCD" modalias.
> > > > > > >>
> > > > > > > IMO, this is not the proper fix for the modalias problem because
> > > > > > > ACPI
> > > > > > > enumerated I2C device may have compatible ids.
> > > > > > > Instead, we should export all the compatible ids as the modules
> > > > > > > alias of
> > > > > > > the ACPI enumerated I2C device.
> > > > > > >
> > > > > > > can you please take a look at the patch I sent out earlier?
> > > > > > > https://patchwork.kernel.org/patch/3034991/
> > > > > > > https://patchwork.kernel.org/patch/3035041/
> > > > > > > https://patchwork.kernel.org/patch/3035021/
> > > > > > I see. This makes sense as it avoids that same device has two
> > > > > > different
> > > > > > modaliases from both acpi and other subsystem.
> > > > > >
> > > > > > How about modalias nodes in sysfs, should they also reflect what is
> > > > > > matching uvent?
> > > > > >
> > > > > good catch, will fix "modalias" as well in next version.
> > > >
> > > > Hi,
> > > >
> > > > I have a question about the device "uevent" and "modalias" sysfs
> > > > attributes.
> > > > what is the relationship between these two?
> > > > Am I right to say that, if there is the "MODALIAS" field in uevent file,
> > > > this field must be consistent with the content in "modalias" attribute?
> >
> > Well, if it isn't, it's pretty pointless, right?
> >
> > > > I checked the code in drivers/base/platform.c,
> > > > static ssize_t modalias_show(struct device *dev, struct device_attribute
> > > > *a,
> > > > char *buf)
> > > > {
> > > > struct platform_device *pdev = to_platform_device(dev);
> > > > int len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
> > > >
> > > > return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
> > > > }
> > > >
> > > > static int platform_uevent(struct device *dev, struct kobj_uevent_env
> > > > *env)
> > > > {
> > > > struct platform_device *pdev = to_platform_device(dev);
> > > > int rc;
> > > >
> > > > /* Some devices have extra OF data and an OF-style MODALIAS */
> > > > rc = of_device_uevent_modalias(dev, env);
> > > > if (rc != -ENODEV)
> > > > return rc;
> > > >
> > > > add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX,
> > > > pdev->name);
> > > > return 0;
> > > > }
> > > >
> > > > This means that the OF-style MODALIAS is not shown in "modalias" sysfs
> > > > attribute.
> > > > is this a bug?
> > >
> > > I would consider that as a bug, but I'm not sure what the recommended
> > > practice
> > > is. Greg?
> >
> > I have no idea how the OF stuff is working, and honestly, I really have
> > no wish to ever know anything about it. Especially when it comes to
> > platform devices/drivers, something that I personally hate and wish
> > would be deleted.
> >
> > So go ask the OF maintainers/developers, this is their domain :)
>
> Well, OK. Whom in particular?
The "OPEN FIRMWARE AND FLATTENED DEVICE TREE" entries in MAINTAINERS?
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: Fix modalias for ACPI enumerated I2C devices
On Tue, Oct 15, 2013 at 10:37:02PM +0200, Rafael J. Wysocki wrote:
> On Tuesday, October 15, 2013 07:44:44 PM Zhang Rui wrote:
> > On Mon, 2013-10-14 at 20:47 +0800, Zhang Rui wrote:
> > > On Mon, 2013-10-14 at 14:18 +0300, Jarkko Nikula wrote:
> > > > On 10/14/2013 12:23 PM, Zhang Rui wrote:
> > > > > Hi,
> > > > >
> > > > > On Thu, 2013-10-10 at 17:17 +0300, Jarkko Nikula wrote:
> > > > >> There is a minor fault about ACPI enumerated I2C devices with their
> > > > >> modalias
> > > > >> attribute. Now modalias is set by device instance not by hardware ID.
> > > > >> For example "i2c:INTABCD:00", "i2c:INTABCD:01" etc.
> > > > >>
> > > > >> This means each device instance gets different modalias which does
> > > > >> match
> > > > >> with generated modules.alias. Currently this is not problem as
> > > > >> matching can
> > > > >> happen also with "acpi:INTABCD" modalias.
> > > > >>
> > > > > IMO, this is not the proper fix for the modalias problem because ACPI
> > > > > enumerated I2C device may have compatible ids.
> > > > > Instead, we should export all the compatible ids as the modules alias
> > > > > of
> > > > > the ACPI enumerated I2C device.
> > > > >
> > > > > can you please take a look at the patch I sent out earlier?
> > > > > https://patchwork.kernel.org/patch/3034991/
> > > > > https://patchwork.kernel.org/patch/3035041/
> > > > > https://patchwork.kernel.org/patch/3035021/
> > > > I see. This makes sense as it avoids that same device has two different
> > > > modaliases from both acpi and other subsystem.
> > > >
> > > > How about modalias nodes in sysfs, should they also reflect what is
> > > > matching uvent?
> > > >
> > > good catch, will fix "modalias" as well in next version.
> >
> > Hi,
> >
> > I have a question about the device "uevent" and "modalias" sysfs
> > attributes.
> > what is the relationship between these two?
> > Am I right to say that, if there is the "MODALIAS" field in uevent file,
> > this field must be consistent with the content in "modalias" attribute?
Well, if it isn't, it's pretty pointless, right?
> > I checked the code in drivers/base/platform.c,
> > static ssize_t modalias_show(struct device *dev, struct device_attribute
> > *a,
> > char *buf)
> > {
> > struct platform_device *pdev = to_platform_device(dev);
> > int len = snprintf(buf, PAGE_SIZE, "platform:%s\n", pdev->name);
> >
> > return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
> > }
> >
> > static int platform_uevent(struct device *dev, struct kobj_uevent_env
> > *env)
> > {
> > struct platform_device *pdev = to_platform_device(dev);
> > int rc;
> >
> > /* Some devices have extra OF data and an OF-style MODALIAS */
> > rc = of_device_uevent_modalias(dev, env);
> > if (rc != -ENODEV)
> > return rc;
> >
> > add_uevent_var(env, "MODALIAS=%s%s", PLATFORM_MODULE_PREFIX,
> > pdev->name);
> > return 0;
> > }
> >
> > This means that the OF-style MODALIAS is not shown in "modalias" sysfs
> > attribute.
> > is this a bug?
>
> I would consider that as a bug, but I'm not sure what the recommended practice
> is. Greg?
I have no idea how the OF stuff is working, and honestly, I really have
no wish to ever know anything about it. Especially when it comes to
platform devices/drivers, something that I personally hate and wish
would be deleted.
So go ask the OF maintainers/developers, this is their domain :)
sorry,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/9] drivers/misc: convert existing I2C clients driver to use I2C core runtime PM
On Wed, Sep 11, 2013 at 06:32:36PM +0300, Mika Westerberg wrote: > The I2C core now prepares runtime PM on behalf of the I2C client device, so > only thing the driver needs to do is to call pm_runtime_put() at the end of > its ->probe(). > > This patch converts I2C client drivers under drivers/misc to use this > model. > > Signed-off-by: Mika Westerberg Acked-by: Greg Kroah-Hartman -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: remove CONFIG_HOTPLUG ifdefs
On Tue, Apr 09, 2013 at 09:46:39AM +0800, Yijing Wang wrote: > CONFIG_HOTPLUG is going away as an option, cleanup CONFIG_HOTPLUG > ifdefs in i2c files. > > Signed-off-by: Yijing Wang > Cc: Greg Kroah-Hartman > Cc: Bill Pemberton Acked-by: Greg Kroah-Hartman -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] MAINTAINERS: i2c: 7 years, this is it
On Mon, Nov 05, 2012 at 05:05:54PM +0100, Jean Delvare wrote: > I have been maintaining the i2c subsystem for 7 years now, it's about > time to let someone else take over. Just before I leave, I would like > to thank several individuals who made this possible at all: > > * Greg Kroah-Hartman, for his faith in my potential subsystem > maintainer skills. Greg, I hope I met your expectations. You've more than met them, you've done a wonderful job. Thanks so much for the work you have done over the past 7 years, it's been very much appreciated. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
