Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Tue, Feb 16, 2021 at 7:05 PM Guenter Roeck wrote: > > On Tue, Feb 16, 2021 at 06:39:55PM -0800, Saravana Kannan wrote: > > On Wed, Feb 10, 2021 at 1:21 PM Guenter Roeck wrote: > > > > > > On 2/10/21 12:52 PM, Saravana Kannan wrote: > > > > On Wed, Feb 10, 2021 at 7:10 AM Guenter Roeck > > > > wrote: > > > >> > > > >> On 2/10/21 12:20 AM, Saravana Kannan wrote: > > > >>> On Tue, Feb 9, 2021 at 9:54 PM Guenter Roeck > > > >>> wrote: > > > > > > On Thu, Dec 17, 2020 at 07:17:03PM -0800, Saravana Kannan wrote: > > > > Cyclic dependencies in some firmware was one of the last remaining > > > > reasons fw_devlink=on couldn't be set by default. Now that cyclic > > > > dependencies don't block probing, set fw_devlink=on by default. > > > > > > > > Setting fw_devlink=on by default brings a bunch of benefits > > > > (currently, > > > > only for systems with device tree firmware): > > > > * Significantly cuts down deferred probes. > > > > * Device probe is effectively attempted in graph order. > > > > * Makes it much easier to load drivers as modules without having to > > > > worry about functional dependencies between modules (depmod is > > > > still > > > > needed for symbol dependencies). > > > > > > > > If this patch prevents some devices from probing, it's very likely > > > > due > > > > to the system having one or more device drivers that "probe"/set up > > > > a > > > > device (DT node with compatible property) without creating a struct > > > > device for it. If we hit such cases, the device drivers need to be > > > > fixed so that they populate struct devices and probe them like > > > > normal > > > > device drivers so that the driver core is aware of the devices and > > > > their > > > > status. See [1] for an example of such a case. > > > > > > > > [1] - > > > > https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/ > > > > Signed-off-by: Saravana Kannan > > > > > > This patch breaks nios2 boot tests in qemu. The system gets stuck > > > when > > > trying to reboot. Reverting this patch fixes the problem. Bisect log > > > is attached. > > > >>> > > > >>> Thanks for the report Guenter. Can you please try this series? > > > >>> https://lore.kernel.org/lkml/20210205222644.2357303-1-sarava...@google.com/ > > > >>> > > > >> > > > >> Not this week. I have lots of reviews to complete before the end of > > > >> the week, > > > >> with the 5.12 commit window coming up. > > > > > > > > Ok. By next week, all the fixes should be in linux-next too. So it > > > > should be easier if you choose to test. > > > > > > > >> Given the number of problems observed, I personally think that it is > > > >> way > > > >> too early for this patch. We'll have no end of problems if it is > > > >> applied > > > >> to the upstream kernel in the next commit window. Of course, that is > > > >> just > > > >> my personal opinion. > > > > > > > > You had said "with 115 of 430 boot tests failing in -next" earlier. > > > > Just to be sure I understand it right, you are not saying this patch > > > > caused them all right? You are just saying that 115 general boot > > > > failures that might mask fw_devlink issues in some of them, right? > > > > > > > > > > Correct. > > > > Is it right to assume [1] fixed all known boot issues due to fw_devlink=on? > > [1] - > > https://lore.kernel.org/lkml/20210215224258.1231449-1-sarava...@google.com/ > > > > I honestly don't know. Current status of -next in my tests is: > > Build results: > total: 149 pass: 144 fail: 5 > Qemu test results: > total: 432 pass: 371 fail: 61 > > This is for next-20210216. Newly introduced failures keep popping up. Some > of the failures have been persistent for weeks, so it is all but impossible > to say if affected platforms experience more than one failure. > > Also, please keep in mind that my boot tests are very shallow, along the > line of "it boots, therefore it works". It only tests hardware which is > emulated by qemu and is needed for booting. It tests probably much less > than 1% of driver code. It can and should not be used for any useful > fw_devlink related test coverage. Agreed. I'm not using this for fw_devlink=on test coverage. Just checking to make sure I've addressed any issues you've seen. FYI, you can change it at runtime using the kernel commandline param fw_devlink=permissive. So, you don't have to build all these kernels again to test if fw_devlink=on is making things worse. -Saravana
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Tue, Feb 16, 2021 at 06:39:55PM -0800, Saravana Kannan wrote: > On Wed, Feb 10, 2021 at 1:21 PM Guenter Roeck wrote: > > > > On 2/10/21 12:52 PM, Saravana Kannan wrote: > > > On Wed, Feb 10, 2021 at 7:10 AM Guenter Roeck wrote: > > >> > > >> On 2/10/21 12:20 AM, Saravana Kannan wrote: > > >>> On Tue, Feb 9, 2021 at 9:54 PM Guenter Roeck wrote: > > > > On Thu, Dec 17, 2020 at 07:17:03PM -0800, Saravana Kannan wrote: > > > Cyclic dependencies in some firmware was one of the last remaining > > > reasons fw_devlink=on couldn't be set by default. Now that cyclic > > > dependencies don't block probing, set fw_devlink=on by default. > > > > > > Setting fw_devlink=on by default brings a bunch of benefits > > > (currently, > > > only for systems with device tree firmware): > > > * Significantly cuts down deferred probes. > > > * Device probe is effectively attempted in graph order. > > > * Makes it much easier to load drivers as modules without having to > > > worry about functional dependencies between modules (depmod is still > > > needed for symbol dependencies). > > > > > > If this patch prevents some devices from probing, it's very likely due > > > to the system having one or more device drivers that "probe"/set up a > > > device (DT node with compatible property) without creating a struct > > > device for it. If we hit such cases, the device drivers need to be > > > fixed so that they populate struct devices and probe them like normal > > > device drivers so that the driver core is aware of the devices and > > > their > > > status. See [1] for an example of such a case. > > > > > > [1] - > > > https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/ > > > Signed-off-by: Saravana Kannan > > > > This patch breaks nios2 boot tests in qemu. The system gets stuck when > > trying to reboot. Reverting this patch fixes the problem. Bisect log > > is attached. > > >>> > > >>> Thanks for the report Guenter. Can you please try this series? > > >>> https://lore.kernel.org/lkml/20210205222644.2357303-1-sarava...@google.com/ > > >>> > > >> > > >> Not this week. I have lots of reviews to complete before the end of the > > >> week, > > >> with the 5.12 commit window coming up. > > > > > > Ok. By next week, all the fixes should be in linux-next too. So it > > > should be easier if you choose to test. > > > > > >> Given the number of problems observed, I personally think that it is way > > >> too early for this patch. We'll have no end of problems if it is applied > > >> to the upstream kernel in the next commit window. Of course, that is just > > >> my personal opinion. > > > > > > You had said "with 115 of 430 boot tests failing in -next" earlier. > > > Just to be sure I understand it right, you are not saying this patch > > > caused them all right? You are just saying that 115 general boot > > > failures that might mask fw_devlink issues in some of them, right? > > > > > > > Correct. > > Is it right to assume [1] fixed all known boot issues due to fw_devlink=on? > [1] - > https://lore.kernel.org/lkml/20210215224258.1231449-1-sarava...@google.com/ > I honestly don't know. Current status of -next in my tests is: Build results: total: 149 pass: 144 fail: 5 Qemu test results: total: 432 pass: 371 fail: 61 This is for next-20210216. Newly introduced failures keep popping up. Some of the failures have been persistent for weeks, so it is all but impossible to say if affected platforms experience more than one failure. Also, please keep in mind that my boot tests are very shallow, along the line of "it boots, therefore it works". It only tests hardware which is emulated by qemu and is needed for booting. It tests probably much less than 1% of driver code. It can and should not be used for any useful fw_devlink related test coverage. Thanks, Guenter
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Wed, Feb 10, 2021 at 1:21 PM Guenter Roeck wrote: > > On 2/10/21 12:52 PM, Saravana Kannan wrote: > > On Wed, Feb 10, 2021 at 7:10 AM Guenter Roeck wrote: > >> > >> On 2/10/21 12:20 AM, Saravana Kannan wrote: > >>> On Tue, Feb 9, 2021 at 9:54 PM Guenter Roeck wrote: > > On Thu, Dec 17, 2020 at 07:17:03PM -0800, Saravana Kannan wrote: > > Cyclic dependencies in some firmware was one of the last remaining > > reasons fw_devlink=on couldn't be set by default. Now that cyclic > > dependencies don't block probing, set fw_devlink=on by default. > > > > Setting fw_devlink=on by default brings a bunch of benefits (currently, > > only for systems with device tree firmware): > > * Significantly cuts down deferred probes. > > * Device probe is effectively attempted in graph order. > > * Makes it much easier to load drivers as modules without having to > > worry about functional dependencies between modules (depmod is still > > needed for symbol dependencies). > > > > If this patch prevents some devices from probing, it's very likely due > > to the system having one or more device drivers that "probe"/set up a > > device (DT node with compatible property) without creating a struct > > device for it. If we hit such cases, the device drivers need to be > > fixed so that they populate struct devices and probe them like normal > > device drivers so that the driver core is aware of the devices and their > > status. See [1] for an example of such a case. > > > > [1] - > > https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/ > > Signed-off-by: Saravana Kannan > > This patch breaks nios2 boot tests in qemu. The system gets stuck when > trying to reboot. Reverting this patch fixes the problem. Bisect log > is attached. > >>> > >>> Thanks for the report Guenter. Can you please try this series? > >>> https://lore.kernel.org/lkml/20210205222644.2357303-1-sarava...@google.com/ > >>> > >> > >> Not this week. I have lots of reviews to complete before the end of the > >> week, > >> with the 5.12 commit window coming up. > > > > Ok. By next week, all the fixes should be in linux-next too. So it > > should be easier if you choose to test. > > > >> Given the number of problems observed, I personally think that it is way > >> too early for this patch. We'll have no end of problems if it is applied > >> to the upstream kernel in the next commit window. Of course, that is just > >> my personal opinion. > > > > You had said "with 115 of 430 boot tests failing in -next" earlier. > > Just to be sure I understand it right, you are not saying this patch > > caused them all right? You are just saying that 115 general boot > > failures that might mask fw_devlink issues in some of them, right? > > > > Correct. Is it right to assume [1] fixed all known boot issues due to fw_devlink=on? [1] - https://lore.kernel.org/lkml/20210215224258.1231449-1-sarava...@google.com/ -Saravana
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On 2/10/21 12:52 PM, Saravana Kannan wrote: > On Wed, Feb 10, 2021 at 7:10 AM Guenter Roeck wrote: >> >> On 2/10/21 12:20 AM, Saravana Kannan wrote: >>> On Tue, Feb 9, 2021 at 9:54 PM Guenter Roeck wrote: On Thu, Dec 17, 2020 at 07:17:03PM -0800, Saravana Kannan wrote: > Cyclic dependencies in some firmware was one of the last remaining > reasons fw_devlink=on couldn't be set by default. Now that cyclic > dependencies don't block probing, set fw_devlink=on by default. > > Setting fw_devlink=on by default brings a bunch of benefits (currently, > only for systems with device tree firmware): > * Significantly cuts down deferred probes. > * Device probe is effectively attempted in graph order. > * Makes it much easier to load drivers as modules without having to > worry about functional dependencies between modules (depmod is still > needed for symbol dependencies). > > If this patch prevents some devices from probing, it's very likely due > to the system having one or more device drivers that "probe"/set up a > device (DT node with compatible property) without creating a struct > device for it. If we hit such cases, the device drivers need to be > fixed so that they populate struct devices and probe them like normal > device drivers so that the driver core is aware of the devices and their > status. See [1] for an example of such a case. > > [1] - > https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/ > Signed-off-by: Saravana Kannan This patch breaks nios2 boot tests in qemu. The system gets stuck when trying to reboot. Reverting this patch fixes the problem. Bisect log is attached. >>> >>> Thanks for the report Guenter. Can you please try this series? >>> https://lore.kernel.org/lkml/20210205222644.2357303-1-sarava...@google.com/ >>> >> >> Not this week. I have lots of reviews to complete before the end of the week, >> with the 5.12 commit window coming up. > > Ok. By next week, all the fixes should be in linux-next too. So it > should be easier if you choose to test. > >> Given the number of problems observed, I personally think that it is way >> too early for this patch. We'll have no end of problems if it is applied >> to the upstream kernel in the next commit window. Of course, that is just >> my personal opinion. > > You had said "with 115 of 430 boot tests failing in -next" earlier. > Just to be sure I understand it right, you are not saying this patch > caused them all right? You are just saying that 115 general boot > failures that might mask fw_devlink issues in some of them, right? > Correct. Guenter
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Wed, Feb 10, 2021 at 7:10 AM Guenter Roeck wrote: > > On 2/10/21 12:20 AM, Saravana Kannan wrote: > > On Tue, Feb 9, 2021 at 9:54 PM Guenter Roeck wrote: > >> > >> On Thu, Dec 17, 2020 at 07:17:03PM -0800, Saravana Kannan wrote: > >>> Cyclic dependencies in some firmware was one of the last remaining > >>> reasons fw_devlink=on couldn't be set by default. Now that cyclic > >>> dependencies don't block probing, set fw_devlink=on by default. > >>> > >>> Setting fw_devlink=on by default brings a bunch of benefits (currently, > >>> only for systems with device tree firmware): > >>> * Significantly cuts down deferred probes. > >>> * Device probe is effectively attempted in graph order. > >>> * Makes it much easier to load drivers as modules without having to > >>> worry about functional dependencies between modules (depmod is still > >>> needed for symbol dependencies). > >>> > >>> If this patch prevents some devices from probing, it's very likely due > >>> to the system having one or more device drivers that "probe"/set up a > >>> device (DT node with compatible property) without creating a struct > >>> device for it. If we hit such cases, the device drivers need to be > >>> fixed so that they populate struct devices and probe them like normal > >>> device drivers so that the driver core is aware of the devices and their > >>> status. See [1] for an example of such a case. > >>> > >>> [1] - > >>> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/ > >>> Signed-off-by: Saravana Kannan > >> > >> This patch breaks nios2 boot tests in qemu. The system gets stuck when > >> trying to reboot. Reverting this patch fixes the problem. Bisect log > >> is attached. > > > > Thanks for the report Guenter. Can you please try this series? > > https://lore.kernel.org/lkml/20210205222644.2357303-1-sarava...@google.com/ > > > > Not this week. I have lots of reviews to complete before the end of the week, > with the 5.12 commit window coming up. Ok. By next week, all the fixes should be in linux-next too. So it should be easier if you choose to test. > Given the number of problems observed, I personally think that it is way > too early for this patch. We'll have no end of problems if it is applied > to the upstream kernel in the next commit window. Of course, that is just > my personal opinion. You had said "with 115 of 430 boot tests failing in -next" earlier. Just to be sure I understand it right, you are not saying this patch caused them all right? You are just saying that 115 general boot failures that might mask fw_devlink issues in some of them, right? Thanks, Saravana
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On 2/10/21 12:20 AM, Saravana Kannan wrote: > On Tue, Feb 9, 2021 at 9:54 PM Guenter Roeck wrote: >> >> On Thu, Dec 17, 2020 at 07:17:03PM -0800, Saravana Kannan wrote: >>> Cyclic dependencies in some firmware was one of the last remaining >>> reasons fw_devlink=on couldn't be set by default. Now that cyclic >>> dependencies don't block probing, set fw_devlink=on by default. >>> >>> Setting fw_devlink=on by default brings a bunch of benefits (currently, >>> only for systems with device tree firmware): >>> * Significantly cuts down deferred probes. >>> * Device probe is effectively attempted in graph order. >>> * Makes it much easier to load drivers as modules without having to >>> worry about functional dependencies between modules (depmod is still >>> needed for symbol dependencies). >>> >>> If this patch prevents some devices from probing, it's very likely due >>> to the system having one or more device drivers that "probe"/set up a >>> device (DT node with compatible property) without creating a struct >>> device for it. If we hit such cases, the device drivers need to be >>> fixed so that they populate struct devices and probe them like normal >>> device drivers so that the driver core is aware of the devices and their >>> status. See [1] for an example of such a case. >>> >>> [1] - >>> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/ >>> Signed-off-by: Saravana Kannan >> >> This patch breaks nios2 boot tests in qemu. The system gets stuck when >> trying to reboot. Reverting this patch fixes the problem. Bisect log >> is attached. > > Thanks for the report Guenter. Can you please try this series? > https://lore.kernel.org/lkml/20210205222644.2357303-1-sarava...@google.com/ > Not this week. I have lots of reviews to complete before the end of the week, with the 5.12 commit window coming up. Given the number of problems observed, I personally think that it is way too early for this patch. We'll have no end of problems if it is applied to the upstream kernel in the next commit window. Of course, that is just my personal opinion. Thanks, Guenter
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Tue, Feb 9, 2021 at 9:54 PM Guenter Roeck wrote: > > On Thu, Dec 17, 2020 at 07:17:03PM -0800, Saravana Kannan wrote: > > Cyclic dependencies in some firmware was one of the last remaining > > reasons fw_devlink=on couldn't be set by default. Now that cyclic > > dependencies don't block probing, set fw_devlink=on by default. > > > > Setting fw_devlink=on by default brings a bunch of benefits (currently, > > only for systems with device tree firmware): > > * Significantly cuts down deferred probes. > > * Device probe is effectively attempted in graph order. > > * Makes it much easier to load drivers as modules without having to > > worry about functional dependencies between modules (depmod is still > > needed for symbol dependencies). > > > > If this patch prevents some devices from probing, it's very likely due > > to the system having one or more device drivers that "probe"/set up a > > device (DT node with compatible property) without creating a struct > > device for it. If we hit such cases, the device drivers need to be > > fixed so that they populate struct devices and probe them like normal > > device drivers so that the driver core is aware of the devices and their > > status. See [1] for an example of such a case. > > > > [1] - > > https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/ > > Signed-off-by: Saravana Kannan > > This patch breaks nios2 boot tests in qemu. The system gets stuck when > trying to reboot. Reverting this patch fixes the problem. Bisect log > is attached. Thanks for the report Guenter. Can you please try this series? https://lore.kernel.org/lkml/20210205222644.2357303-1-sarava...@google.com/ It's in driver-core-testing too if that's easier. -Saravana
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Thu, Dec 17, 2020 at 07:17:03PM -0800, Saravana Kannan wrote: > Cyclic dependencies in some firmware was one of the last remaining > reasons fw_devlink=on couldn't be set by default. Now that cyclic > dependencies don't block probing, set fw_devlink=on by default. > > Setting fw_devlink=on by default brings a bunch of benefits (currently, > only for systems with device tree firmware): > * Significantly cuts down deferred probes. > * Device probe is effectively attempted in graph order. > * Makes it much easier to load drivers as modules without having to > worry about functional dependencies between modules (depmod is still > needed for symbol dependencies). > > If this patch prevents some devices from probing, it's very likely due > to the system having one or more device drivers that "probe"/set up a > device (DT node with compatible property) without creating a struct > device for it. If we hit such cases, the device drivers need to be > fixed so that they populate struct devices and probe them like normal > device drivers so that the driver core is aware of the devices and their > status. See [1] for an example of such a case. > > [1] - > https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/ > Signed-off-by: Saravana Kannan This patch breaks nios2 boot tests in qemu. The system gets stuck when trying to reboot. Reverting this patch fixes the problem. Bisect log is attached. It may also break a variety of other boot tests, but with 115 of 430 boot tests failing in -next it is difficult to identify all culprits. Guenter --- Bisect log: # bad: [a4bfd8d46ac357c12529e4eebb6c89502b03ecc9] Add linux-next specific files for 20210209 # good: [92bf22614b21a2706f4993b278017e437f7785b3] Linux 5.11-rc7 git bisect start 'HEAD' 'v5.11-rc7' # good: [a8eb921ba7e8e77d994a1c6c69c8ef08456ecf53] Merge remote-tracking branch 'crypto/master' git bisect good a8eb921ba7e8e77d994a1c6c69c8ef08456ecf53 # good: [21d507c41bdf83f6afc0e02976e43c10badfc6cd] Merge remote-tracking branch 'spi/for-next' git bisect good 21d507c41bdf83f6afc0e02976e43c10badfc6cd # bad: [30cd4c688a3bcf324f011d7716044b1a4681efc1] Merge remote-tracking branch 'soundwire/next' git bisect bad 30cd4c688a3bcf324f011d7716044b1a4681efc1 # good: [c43d2173d3eb4047bb62a7a393a298a1032cce18] Merge remote-tracking branch 'drivers-x86/for-next' git bisect good c43d2173d3eb4047bb62a7a393a298a1032cce18 # bad: [4dd66c506de68a592f2dd4ef64cc9b0d7c0f3117] Merge remote-tracking branch 'usb-chipidea-next/for-usb-next' git bisect bad 4dd66c506de68a592f2dd4ef64cc9b0d7c0f3117 # good: [29b01295a829fba7399ee84afff4e64660e49f04] usb: typec: Add typec_partner_set_pd_revision git bisect good 29b01295a829fba7399ee84afff4e64660e49f04 # bad: [dac8ab120e531bf7c358b85750338e1b3d3ca0b9] Merge remote-tracking branch 'usb/usb-next' git bisect bad dac8ab120e531bf7c358b85750338e1b3d3ca0b9 # good: [678481467d2e1460a49e626d8e9ba0c7e9742f53] usb: dwc3: core: Check maximum_speed SSP genXxY git bisect good 678481467d2e1460a49e626d8e9ba0c7e9742f53 # good: [5d3d0a61479847a8729ffdda33867f6b3443c15f] Merge remote-tracking branch 'ipmi/for-next' git bisect good 5d3d0a61479847a8729ffdda33867f6b3443c15f # bad: [e13f5b7a130f7b6d4d34be27a87393890b5ee2ba] of: property: Add fw_devlink support for "gpio" and "gpios" binding git bisect bad e13f5b7a130f7b6d4d34be27a87393890b5ee2ba # good: [b0e2fa4f611bb9ab22928605d5b1c7fd44e73955] driver core: Handle cycles in device links created by fw_devlink git bisect good b0e2fa4f611bb9ab22928605d5b1c7fd44e73955 # bad: [0fab972eef49ef8d30eb91d6bd98861122d083d1] drivers: core: Detach device from power domain on shutdown git bisect bad 0fab972eef49ef8d30eb91d6bd98861122d083d1 # bad: [e590474768f1cc04852190b61dec692411b22e2a] driver core: Set fw_devlink=on by default git bisect bad e590474768f1cc04852190b61dec692411b22e2a # good: [c13b827927112ba6170bea31c638a8573c127461] driver core: fw_devlink_relax_cycle() can be static git bisect good c13b827927112ba6170bea31c638a8573c127461 # first bad commit: [e590474768f1cc04852190b61dec692411b22e2a] driver core: Set fw_devlink=on by default
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Thu, Jan 28, 2021 at 2:59 AM wrote:
>
> Hi, Saravana,
>
> On 1/25/21 8:16 PM, Saravana Kannan wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> > content is safe
> >
> > On Mon, Jan 25, 2021 at 9:05 AM wrote:
> >>
> >> Hi, Saravana,
> >>
> >> On 12/18/20 5:17 AM, Saravana Kannan wrote:
> >>> Cyclic dependencies in some firmware was one of the last remaining
> >>> reasons fw_devlink=on couldn't be set by default. Now that cyclic
> >>> dependencies don't block probing, set fw_devlink=on by default.
> >>>
> >>> Setting fw_devlink=on by default brings a bunch of benefits (currently,
> >>> only for systems with device tree firmware):
> >>> * Significantly cuts down deferred probes.
> >>> * Device probe is effectively attempted in graph order.
> >>> * Makes it much easier to load drivers as modules without having to
> >>> worry about functional dependencies between modules (depmod is still
> >>> needed for symbol dependencies).
> >>>
> >>> If this patch prevents some devices from probing, it's very likely due
> >>> to the system having one or more device drivers that "probe"/set up a
> >>> device (DT node with compatible property) without creating a struct
> >>> device for it. If we hit such cases, the device drivers need to be
> >>> fixed so that they populate struct devices and probe them like normal
> >>> device drivers so that the driver core is aware of the devices and their
> >>> status. See [1] for an example of such a case.
> >>>
> >>> [1] -
> >>> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> >>> Signed-off-by: Saravana Kannan
> >>
> >> next-20210125 fails to boot on at91 sama5d2 platforms. No output is
> >> seen, unless earlyprintk is enabled.
> >>
> >> I have bisected this to commit e590474768f1cc04 ("driver core: Set
> >> fw_devlink=on by default").
> >>
> >> I've attached a log that I'm seeing on a sama5d2_xplained (sama5_defconfig
> >> and arch/arm/boot/dts/at91-sama5d2_xplained.dts). I enabled the
> >> following logs:
> >> 1. The ones in device_links_check_suppliers()
> >> 2. The ones in device_link_add()
> >> 3. initcall_debug=1
> >>
> >> There seem to be some probe fails due to the pmc supplier not being ready:
> >> calling at_xdmac_init+0x0/0x18 @ 1
> >> platform f001.dma-controller: probe deferral - supplier f0014000.pmc
> >> not ready
> >> platform f0004000.dma-controller: probe deferral - supplier f0014000.pmc
> >> not ready
> >> initcall at_xdmac_init+0x0/0x18 returned -19 after 19531 usecs
> >>
> >> calling udc_driver_init+0x0/0x18 @ 1
> >> platform 30.gadget: probe deferral - supplier f0014000.pmc not ready
> >> initcall udc_driver_init+0x0/0x18 returned -19 after 7524 usecs
> >>
> >> There are others too. I'm checking them.
> >
> > Thanks Tudor. I'll look into this within a few days. I'm also looking
> > into coming up with a more generic solution.
> >
>
> I've sent a patch addressing this at:
> https://lore.kernel.org/lkml/20210128104446.164269-1-tudor.amba...@microchip.com/T/#u
>
> Can you please take a look?
Thanks for taking a look at this. I responded in that thread.
-Saravana
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi, Saravana,
On 1/25/21 8:16 PM, Saravana Kannan wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On Mon, Jan 25, 2021 at 9:05 AM wrote:
>>
>> Hi, Saravana,
>>
>> On 12/18/20 5:17 AM, Saravana Kannan wrote:
>>> Cyclic dependencies in some firmware was one of the last remaining
>>> reasons fw_devlink=on couldn't be set by default. Now that cyclic
>>> dependencies don't block probing, set fw_devlink=on by default.
>>>
>>> Setting fw_devlink=on by default brings a bunch of benefits (currently,
>>> only for systems with device tree firmware):
>>> * Significantly cuts down deferred probes.
>>> * Device probe is effectively attempted in graph order.
>>> * Makes it much easier to load drivers as modules without having to
>>> worry about functional dependencies between modules (depmod is still
>>> needed for symbol dependencies).
>>>
>>> If this patch prevents some devices from probing, it's very likely due
>>> to the system having one or more device drivers that "probe"/set up a
>>> device (DT node with compatible property) without creating a struct
>>> device for it. If we hit such cases, the device drivers need to be
>>> fixed so that they populate struct devices and probe them like normal
>>> device drivers so that the driver core is aware of the devices and their
>>> status. See [1] for an example of such a case.
>>>
>>> [1] -
>>> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
>>> Signed-off-by: Saravana Kannan
>>
>> next-20210125 fails to boot on at91 sama5d2 platforms. No output is
>> seen, unless earlyprintk is enabled.
>>
>> I have bisected this to commit e590474768f1cc04 ("driver core: Set
>> fw_devlink=on by default").
>>
>> I've attached a log that I'm seeing on a sama5d2_xplained (sama5_defconfig
>> and arch/arm/boot/dts/at91-sama5d2_xplained.dts). I enabled the
>> following logs:
>> 1. The ones in device_links_check_suppliers()
>> 2. The ones in device_link_add()
>> 3. initcall_debug=1
>>
>> There seem to be some probe fails due to the pmc supplier not being ready:
>> calling at_xdmac_init+0x0/0x18 @ 1
>> platform f001.dma-controller: probe deferral - supplier f0014000.pmc not
>> ready
>> platform f0004000.dma-controller: probe deferral - supplier f0014000.pmc not
>> ready
>> initcall at_xdmac_init+0x0/0x18 returned -19 after 19531 usecs
>>
>> calling udc_driver_init+0x0/0x18 @ 1
>> platform 30.gadget: probe deferral - supplier f0014000.pmc not ready
>> initcall udc_driver_init+0x0/0x18 returned -19 after 7524 usecs
>>
>> There are others too. I'm checking them.
>
> Thanks Tudor. I'll look into this within a few days. I'm also looking
> into coming up with a more generic solution.
>
I've sent a patch addressing this at:
https://lore.kernel.org/lkml/20210128104446.164269-1-tudor.amba...@microchip.com/T/#u
Can you please take a look?
Cheers,
ta
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi Saravana,
On Tue, Jan 26, 2021 at 12:31 AM Saravana Kannan wrote:
> On Thu, Jan 21, 2021 at 8:04 AM Geert Uytterhoeven
> wrote:
> > On Wed, Jan 20, 2021 at 6:23 PM Saravana Kannan
> > wrote:
> > > On Wed, Jan 20, 2021 at 6:27 AM Geert Uytterhoeven
> > > wrote:
> > > > On Wed, Jan 20, 2021 at 10:40 AM Geert Uytterhoeven
> > > > wrote:
> > > > > On Tue, Jan 19, 2021 at 10:51 PM Saravana Kannan
> > > > > wrote:
> > > > > > On Tue, Jan 19, 2021 at 10:08 AM Saravana Kannan
> > > > > > wrote:
> > > > > > > On Tue, Jan 19, 2021 at 1:05 AM Geert Uytterhoeven
> > > > > > > wrote:
> > > > > > > > On Mon, Jan 18, 2021 at 10:19 PM Saravana Kannan
> > > > > > > > wrote:
> > > > > > > > > On Mon, Jan 18, 2021 at 11:16 AM Geert Uytterhoeven
> > > > > > > > > wrote:
> > > > > > > > > > On Mon, Jan 18, 2021 at 6:59 PM Marc Zyngier
> > > > > > > > > > wrote:
> > > > > > > > > > > On 2021-01-18 17:39, Geert Uytterhoeven wrote:
> > > > > > > > > > > > On Fri, Dec 18, 2020 at 4:34 AM Saravana Kannan
> > > > > > > > > > > >
> > > > > > > > > > > > wrote:
> > > > > > > > > > > >> Cyclic dependencies in some firmware was one of the
> > > > > > > > > > > >> last remaining
> > > > > > > > > > > >> reasons fw_devlink=on couldn't be set by default. Now
> > > > > > > > > > > >> that cyclic
> > > > > > > > > > > >> dependencies don't block probing, set fw_devlink=on by
> > > > > > > > > > > >> default.
> > > > > > > > > > > >>
> > > > > > > > > > > >> Setting fw_devlink=on by default brings a bunch of
> > > > > > > > > > > >> benefits
> > > > > > > > > > > >> (currently,
> > > > > > > > > > > >> only for systems with device tree firmware):
> > > > > > > > > > > >> * Significantly cuts down deferred probes.
> > > > > > > > > > > >> * Device probe is effectively attempted in graph order.
> > > > > > > > > > > >> * Makes it much easier to load drivers as modules
> > > > > > > > > > > >> without having to
> > > > > > > > > > > >> worry about functional dependencies between modules
> > > > > > > > > > > >> (depmod is still
> > > > > > > > > > > >> needed for symbol dependencies).
> > > > > > > > > > > >>
> > > > > > > > > > > >> If this patch prevents some devices from probing, it's
> > > > > > > > > > > >> very likely due
> > > > > > > > > > > >> to the system having one or more device drivers that
> > > > > > > > > > > >> "probe"/set up a
> > > > > > > > > > > >> device (DT node with compatible property) without
> > > > > > > > > > > >> creating a struct
> > > > > > > > > > > >> device for it. If we hit such cases, the device
> > > > > > > > > > > >> drivers need to be
> > > > > > > > > > > >> fixed so that they populate struct devices and probe
> > > > > > > > > > > >> them like normal
> > > > > > > > > > > >> device drivers so that the driver core is aware of the
> > > > > > > > > > > >> devices and
> > > > > > > > > > > >> their
> > > > > > > > > > > >> status. See [1] for an example of such a case.
> > > > > > > > > > > >>
> > > > > > > > > > > >> [1] -
> > > > > > > > > > > >> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> > > > > > > > > > > >> Signed-off-by: Saravana Kannan
> > > > > > > > > > > >
> > > > > > > > > > > > Shimoda-san reported that next-20210111 and later fail
> > > > > > > > > > > > to boot
> > > > > > > > > > > > on Renesas R-Car Gen3 platforms. No output is seen,
> > > > > > > > > > > > unless earlycon
> > > > > > > > > > > > is enabled.
> > > > > > > > > > > >
> > > > > > > > > > > > I have bisected this to commit e590474768f1cc04
> > > > > > > > > > > > ("driver core: Set
> > > > > > > > > > > > fw_devlink=on by default").
> >
> > > > > > You'll need to convert drivers/soc/renesas/rcar-sysc.c into a
> > > > > > platform
> > > > > > driver. You already have a platform device created for it. So just
> > > > > > go
> > > > > > ahead and probe it with a platform driver. See what Marek did here
> > > > > > [1].
> > > > > >
> > > > > > You probably had to implement it as an "initcall based driver"
> > > > > > because you had to play initcall chicken to make sure the PD
> > > > > > hardware
> > > > > > was initialized before the consumers. With fw_devlink=on you won't
> > > > > > have to worry about that. As an added benefit of implementing a
> > > > > > proper
> > > > > > platform driver, you can actually implement runtime PM now, your
> > > > > > suspend/resume would be more robust, etc.
> > > > >
> > > > > On R-Car H1, the system controller driver needs to be active before
> > > > > secondary CPU setup, hence the early_initcall().
> > > > > platform_bus_init() is called after that, so this is gonna need a
> > > > > split
> > > > > initialization. Or a dummy platform driver to make devlinks think
> > > > > everything is fine ;-)
> > >
> > > I was wondering if you could still probe the "not needed by CPU" power
> > > domains (if there are any) as devices. Using driver-core brings you
> > > good things :)
> >
> >
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Thu, Jan 21, 2021 at 8:04 AM Geert Uytterhoeven wrote:
>
> Hi Saravana,
>
> On Wed, Jan 20, 2021 at 6:23 PM Saravana Kannan wrote:
> > On Wed, Jan 20, 2021 at 6:27 AM Geert Uytterhoeven
> > wrote:
> > > On Wed, Jan 20, 2021 at 10:40 AM Geert Uytterhoeven
> > > wrote:
> > > > On Tue, Jan 19, 2021 at 10:51 PM Saravana Kannan
> > > > wrote:
> > > > > On Tue, Jan 19, 2021 at 10:08 AM Saravana Kannan
> > > > > wrote:
> > > > > > On Tue, Jan 19, 2021 at 1:05 AM Geert Uytterhoeven
> > > > > > wrote:
> > > > > > > On Mon, Jan 18, 2021 at 10:19 PM Saravana Kannan
> > > > > > > wrote:
> > > > > > > > On Mon, Jan 18, 2021 at 11:16 AM Geert Uytterhoeven
> > > > > > > > wrote:
> > > > > > > > > On Mon, Jan 18, 2021 at 6:59 PM Marc Zyngier
> > > > > > > > > wrote:
> > > > > > > > > > On 2021-01-18 17:39, Geert Uytterhoeven wrote:
> > > > > > > > > > > On Fri, Dec 18, 2020 at 4:34 AM Saravana Kannan
> > > > > > > > > > >
> > > > > > > > > > > wrote:
> > > > > > > > > > >> Cyclic dependencies in some firmware was one of the last
> > > > > > > > > > >> remaining
> > > > > > > > > > >> reasons fw_devlink=on couldn't be set by default. Now
> > > > > > > > > > >> that cyclic
> > > > > > > > > > >> dependencies don't block probing, set fw_devlink=on by
> > > > > > > > > > >> default.
> > > > > > > > > > >>
> > > > > > > > > > >> Setting fw_devlink=on by default brings a bunch of
> > > > > > > > > > >> benefits
> > > > > > > > > > >> (currently,
> > > > > > > > > > >> only for systems with device tree firmware):
> > > > > > > > > > >> * Significantly cuts down deferred probes.
> > > > > > > > > > >> * Device probe is effectively attempted in graph order.
> > > > > > > > > > >> * Makes it much easier to load drivers as modules
> > > > > > > > > > >> without having to
> > > > > > > > > > >> worry about functional dependencies between modules
> > > > > > > > > > >> (depmod is still
> > > > > > > > > > >> needed for symbol dependencies).
> > > > > > > > > > >>
> > > > > > > > > > >> If this patch prevents some devices from probing, it's
> > > > > > > > > > >> very likely due
> > > > > > > > > > >> to the system having one or more device drivers that
> > > > > > > > > > >> "probe"/set up a
> > > > > > > > > > >> device (DT node with compatible property) without
> > > > > > > > > > >> creating a struct
> > > > > > > > > > >> device for it. If we hit such cases, the device drivers
> > > > > > > > > > >> need to be
> > > > > > > > > > >> fixed so that they populate struct devices and probe
> > > > > > > > > > >> them like normal
> > > > > > > > > > >> device drivers so that the driver core is aware of the
> > > > > > > > > > >> devices and
> > > > > > > > > > >> their
> > > > > > > > > > >> status. See [1] for an example of such a case.
> > > > > > > > > > >>
> > > > > > > > > > >> [1] -
> > > > > > > > > > >> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> > > > > > > > > > >> Signed-off-by: Saravana Kannan
> > > > > > > > > > >
> > > > > > > > > > > Shimoda-san reported that next-20210111 and later fail to
> > > > > > > > > > > boot
> > > > > > > > > > > on Renesas R-Car Gen3 platforms. No output is seen,
> > > > > > > > > > > unless earlycon
> > > > > > > > > > > is enabled.
> > > > > > > > > > >
> > > > > > > > > > > I have bisected this to commit e590474768f1cc04 ("driver
> > > > > > > > > > > core: Set
> > > > > > > > > > > fw_devlink=on by default").
>
> > > > > You'll need to convert drivers/soc/renesas/rcar-sysc.c into a platform
> > > > > driver. You already have a platform device created for it. So just go
> > > > > ahead and probe it with a platform driver. See what Marek did here
> > > > > [1].
> > > > >
> > > > > You probably had to implement it as an "initcall based driver"
> > > > > because you had to play initcall chicken to make sure the PD hardware
> > > > > was initialized before the consumers. With fw_devlink=on you won't
> > > > > have to worry about that. As an added benefit of implementing a proper
> > > > > platform driver, you can actually implement runtime PM now, your
> > > > > suspend/resume would be more robust, etc.
> > > >
> > > > On R-Car H1, the system controller driver needs to be active before
> > > > secondary CPU setup, hence the early_initcall().
> > > > platform_bus_init() is called after that, so this is gonna need a split
> > > > initialization. Or a dummy platform driver to make devlinks think
> > > > everything is fine ;-)
> >
> > I was wondering if you could still probe the "not needed by CPU" power
> > domains (if there are any) as devices. Using driver-core brings you
> > good things :)
>
> 1. That would mean splitting the driver in two parts, looping over the
> tables twice, while everything can just be done in the first pass?
>
> 2. Which "good things" do you have in mind? Making the driver modular?
> Ignoring the dependency for secondary CPU setup on R-Car H1, this
> driver
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Mon, Jan 25, 2021 at 9:05 AM wrote:
>
> Hi, Saravana,
>
> On 12/18/20 5:17 AM, Saravana Kannan wrote:
> > Cyclic dependencies in some firmware was one of the last remaining
> > reasons fw_devlink=on couldn't be set by default. Now that cyclic
> > dependencies don't block probing, set fw_devlink=on by default.
> >
> > Setting fw_devlink=on by default brings a bunch of benefits (currently,
> > only for systems with device tree firmware):
> > * Significantly cuts down deferred probes.
> > * Device probe is effectively attempted in graph order.
> > * Makes it much easier to load drivers as modules without having to
> > worry about functional dependencies between modules (depmod is still
> > needed for symbol dependencies).
> >
> > If this patch prevents some devices from probing, it's very likely due
> > to the system having one or more device drivers that "probe"/set up a
> > device (DT node with compatible property) without creating a struct
> > device for it. If we hit such cases, the device drivers need to be
> > fixed so that they populate struct devices and probe them like normal
> > device drivers so that the driver core is aware of the devices and their
> > status. See [1] for an example of such a case.
> >
> > [1] -
> > https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> > Signed-off-by: Saravana Kannan
>
> next-20210125 fails to boot on at91 sama5d2 platforms. No output is
> seen, unless earlyprintk is enabled.
>
> I have bisected this to commit e590474768f1cc04 ("driver core: Set
> fw_devlink=on by default").
>
> I've attached a log that I'm seeing on a sama5d2_xplained (sama5_defconfig
> and arch/arm/boot/dts/at91-sama5d2_xplained.dts). I enabled the
> following logs:
> 1. The ones in device_links_check_suppliers()
> 2. The ones in device_link_add()
> 3. initcall_debug=1
>
> There seem to be some probe fails due to the pmc supplier not being ready:
> calling at_xdmac_init+0x0/0x18 @ 1
> platform f001.dma-controller: probe deferral - supplier f0014000.pmc not
> ready
> platform f0004000.dma-controller: probe deferral - supplier f0014000.pmc not
> ready
> initcall at_xdmac_init+0x0/0x18 returned -19 after 19531 usecs
>
> calling udc_driver_init+0x0/0x18 @ 1
> platform 30.gadget: probe deferral - supplier f0014000.pmc not ready
> initcall udc_driver_init+0x0/0x18 returned -19 after 7524 usecs
>
> There are others too. I'm checking them.
Thanks Tudor. I'll look into this within a few days. I'm also looking
into coming up with a more generic solution.
-Saravana
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi, Saravana,
On 12/18/20 5:17 AM, Saravana Kannan wrote:
> Cyclic dependencies in some firmware was one of the last remaining
> reasons fw_devlink=on couldn't be set by default. Now that cyclic
> dependencies don't block probing, set fw_devlink=on by default.
>
> Setting fw_devlink=on by default brings a bunch of benefits (currently,
> only for systems with device tree firmware):
> * Significantly cuts down deferred probes.
> * Device probe is effectively attempted in graph order.
> * Makes it much easier to load drivers as modules without having to
> worry about functional dependencies between modules (depmod is still
> needed for symbol dependencies).
>
> If this patch prevents some devices from probing, it's very likely due
> to the system having one or more device drivers that "probe"/set up a
> device (DT node with compatible property) without creating a struct
> device for it. If we hit such cases, the device drivers need to be
> fixed so that they populate struct devices and probe them like normal
> device drivers so that the driver core is aware of the devices and their
> status. See [1] for an example of such a case.
>
> [1] -
> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> Signed-off-by: Saravana Kannan
next-20210125 fails to boot on at91 sama5d2 platforms. No output is
seen, unless earlyprintk is enabled.
I have bisected this to commit e590474768f1cc04 ("driver core: Set
fw_devlink=on by default").
I've attached a log that I'm seeing on a sama5d2_xplained (sama5_defconfig
and arch/arm/boot/dts/at91-sama5d2_xplained.dts). I enabled the
following logs:
1. The ones in device_links_check_suppliers()
2. The ones in device_link_add()
3. initcall_debug=1
There seem to be some probe fails due to the pmc supplier not being ready:
calling at_xdmac_init+0x0/0x18 @ 1
platform f001.dma-controller: probe deferral - supplier f0014000.pmc not
ready
platform f0004000.dma-controller: probe deferral - supplier f0014000.pmc not
ready
initcall at_xdmac_init+0x0/0x18 returned -19 after 19531 usecs
calling udc_driver_init+0x0/0x18 @ 1
platform 30.gadget: probe deferral - supplier f0014000.pmc not ready
initcall udc_driver_init+0x0/0x18 returned -19 after 7524 usecs
There are others too. I'm checking them.
Cheers,
ta
Starting kernel ...
Booting Linux on physical CPU 0x0
Linux version 5.11.0-rc4-next-20210125+ (atudor@atudor-ThinkPad-T470p) (arm-linux-gnueabi-gcc (Linaro GCC 7.5-2019.12) 7.5.0, GNU ld (Linaro_Binutils-2019.12) 2.28.2.20170706) #57 Mon Jan 25 18:36:05 EET 2021
CPU: ARMv7 Processor [410fc051] revision 1 (ARMv7), cr=10c53c7d
CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
OF: fdt: Machine model: Atmel SAMA5D2 Xplained
printk: debug: ignoring loglevel setting.
printk: bootconsole [earlycon0] enabled
Memory policy: Data cache writeback
Zone ranges:
Normal [mem 0x2000-0x3fff]
Movable zone start for each node
Early memory node ranges
node 0: [mem 0x2000-0x3fff]
Initmem setup node 0 [mem 0x2000-0x3fff]
On node 0 totalpages: 131072
Normal zone: 1024 pages used for memmap
Normal zone: 0 pages reserved
Normal zone: 131072 pages, LIFO batch:31
CPU: All CPU(s) started in SVC mode.
pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768
pcpu-alloc: [0] 0
Built 1 zonelists, mobility grouping on. Total pages: 130048
Kernel command line: console=ttyS0,115200 root=/dev/nfs rootfstype=nfs rw rootwait ignore_loglevel earlyprintk initcall_debug nfsroot=192.168.0.120:/nfsroot/2020.04/sama5d2-xplained,tcp,v3 ip=192.168.0.125:192.168.0.120::
Dentry cache hash table entries: 65536 (order: 6, 262144 bytes, linear)
Inode-cache hash table entries: 32768 (order: 5, 131072 bytes, linear)
mem auto-init: stack:off, heap alloc:off, heap free:off
Memory: 506940K/524288K available (8192K kernel code, 281K rwdata, 1896K rodata, 1024K init, 113K bss, 17348K reserved, 0K cma-reserved)
NR_IRQS: 16, nr_irqs: 16, preallocated irqs: 16
L2C-310 ID prefetch enabled, offset 2 lines
L2C-310 dynamic clock gating enabled, standby mode enabled
L2C-310 cache controller enabled, 8 ways, 128 kB
L2C-310: CACHE_ID 0x41c9, AUX_CTRL 0x3602
random: get_random_bytes called from start_kernel+0x33c/0x4c0 with crng_init=0
clocksource: timer@f800c000: mask: 0x max_cycles: 0x, max_idle_ns: 184217874325 ns
sched_clock: 32 bits at 10MHz, resolution 96ns, wraps every 206986376143ns
Switching to timer-based delay loop, resolution 96ns
Console: colour dummy device 80x30
Calibrating delay loop (skipped), value calculated using timer frequency.. 20.75 BogoMIPS (lpj=103750)
pid_max: default: 32768 minimum: 301
Mount-cache hash table entries: 1024 (order: 0, 4096 bytes, linear)
Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes, linear)
CPU: Testing write buffer coherency: ok
calling
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi Saravana,
On Wed, Jan 20, 2021 at 6:23 PM Saravana Kannan wrote:
> On Wed, Jan 20, 2021 at 6:27 AM Geert Uytterhoeven
> wrote:
> > On Wed, Jan 20, 2021 at 10:40 AM Geert Uytterhoeven
> > wrote:
> > > On Tue, Jan 19, 2021 at 10:51 PM Saravana Kannan
> > > wrote:
> > > > On Tue, Jan 19, 2021 at 10:08 AM Saravana Kannan
> > > > wrote:
> > > > > On Tue, Jan 19, 2021 at 1:05 AM Geert Uytterhoeven
> > > > > wrote:
> > > > > > On Mon, Jan 18, 2021 at 10:19 PM Saravana Kannan
> > > > > > wrote:
> > > > > > > On Mon, Jan 18, 2021 at 11:16 AM Geert Uytterhoeven
> > > > > > > wrote:
> > > > > > > > On Mon, Jan 18, 2021 at 6:59 PM Marc Zyngier
> > > > > > > > wrote:
> > > > > > > > > On 2021-01-18 17:39, Geert Uytterhoeven wrote:
> > > > > > > > > > On Fri, Dec 18, 2020 at 4:34 AM Saravana Kannan
> > > > > > > > > >
> > > > > > > > > > wrote:
> > > > > > > > > >> Cyclic dependencies in some firmware was one of the last
> > > > > > > > > >> remaining
> > > > > > > > > >> reasons fw_devlink=on couldn't be set by default. Now that
> > > > > > > > > >> cyclic
> > > > > > > > > >> dependencies don't block probing, set fw_devlink=on by
> > > > > > > > > >> default.
> > > > > > > > > >>
> > > > > > > > > >> Setting fw_devlink=on by default brings a bunch of benefits
> > > > > > > > > >> (currently,
> > > > > > > > > >> only for systems with device tree firmware):
> > > > > > > > > >> * Significantly cuts down deferred probes.
> > > > > > > > > >> * Device probe is effectively attempted in graph order.
> > > > > > > > > >> * Makes it much easier to load drivers as modules without
> > > > > > > > > >> having to
> > > > > > > > > >> worry about functional dependencies between modules
> > > > > > > > > >> (depmod is still
> > > > > > > > > >> needed for symbol dependencies).
> > > > > > > > > >>
> > > > > > > > > >> If this patch prevents some devices from probing, it's
> > > > > > > > > >> very likely due
> > > > > > > > > >> to the system having one or more device drivers that
> > > > > > > > > >> "probe"/set up a
> > > > > > > > > >> device (DT node with compatible property) without creating
> > > > > > > > > >> a struct
> > > > > > > > > >> device for it. If we hit such cases, the device drivers
> > > > > > > > > >> need to be
> > > > > > > > > >> fixed so that they populate struct devices and probe them
> > > > > > > > > >> like normal
> > > > > > > > > >> device drivers so that the driver core is aware of the
> > > > > > > > > >> devices and
> > > > > > > > > >> their
> > > > > > > > > >> status. See [1] for an example of such a case.
> > > > > > > > > >>
> > > > > > > > > >> [1] -
> > > > > > > > > >> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> > > > > > > > > >> Signed-off-by: Saravana Kannan
> > > > > > > > > >
> > > > > > > > > > Shimoda-san reported that next-20210111 and later fail to
> > > > > > > > > > boot
> > > > > > > > > > on Renesas R-Car Gen3 platforms. No output is seen, unless
> > > > > > > > > > earlycon
> > > > > > > > > > is enabled.
> > > > > > > > > >
> > > > > > > > > > I have bisected this to commit e590474768f1cc04 ("driver
> > > > > > > > > > core: Set
> > > > > > > > > > fw_devlink=on by default").
> > > > You'll need to convert drivers/soc/renesas/rcar-sysc.c into a platform
> > > > driver. You already have a platform device created for it. So just go
> > > > ahead and probe it with a platform driver. See what Marek did here
> > > > [1].
> > > >
> > > > You probably had to implement it as an "initcall based driver"
> > > > because you had to play initcall chicken to make sure the PD hardware
> > > > was initialized before the consumers. With fw_devlink=on you won't
> > > > have to worry about that. As an added benefit of implementing a proper
> > > > platform driver, you can actually implement runtime PM now, your
> > > > suspend/resume would be more robust, etc.
> > >
> > > On R-Car H1, the system controller driver needs to be active before
> > > secondary CPU setup, hence the early_initcall().
> > > platform_bus_init() is called after that, so this is gonna need a split
> > > initialization. Or a dummy platform driver to make devlinks think
> > > everything is fine ;-)
>
> I was wondering if you could still probe the "not needed by CPU" power
> domains (if there are any) as devices. Using driver-core brings you
> good things :)
1. That would mean splitting the driver in two parts, looping over the
tables twice, while everything can just be done in the first pass?
2. Which "good things" do you have in mind? Making the driver modular?
Ignoring the dependency for secondary CPU setup on R-Car H1, this
driver could indeed be modular on R-Car Gen2 and Gen3, as long as
the boot loader would pass a ramdisk with the module to the kernel.
The ramdisk could not be loaded in any other way, as all I/O
devices are part of a PM Domain, and thus depend on the SYSC driver.
Note
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Wed, Jan 20, 2021 at 6:27 AM Geert Uytterhoeven wrote:
>
> Hi Saravana,
>
> On Wed, Jan 20, 2021 at 10:40 AM Geert Uytterhoeven
> wrote:
> > On Tue, Jan 19, 2021 at 10:51 PM Saravana Kannan
> > wrote:
> > > On Tue, Jan 19, 2021 at 10:08 AM Saravana Kannan
> > > wrote:
> > > > On Tue, Jan 19, 2021 at 1:05 AM Geert Uytterhoeven
> > > > wrote:
> > > > > On Mon, Jan 18, 2021 at 10:19 PM Saravana Kannan
> > > > > wrote:
> > > > > > On Mon, Jan 18, 2021 at 11:16 AM Geert Uytterhoeven
> > > > > > wrote:
> > > > > > > On Mon, Jan 18, 2021 at 6:59 PM Marc Zyngier
> > > > > > > wrote:
> > > > > > > > On 2021-01-18 17:39, Geert Uytterhoeven wrote:
> > > > > > > > > On Fri, Dec 18, 2020 at 4:34 AM Saravana Kannan
> > > > > > > > >
> > > > > > > > > wrote:
> > > > > > > > >> Cyclic dependencies in some firmware was one of the last
> > > > > > > > >> remaining
> > > > > > > > >> reasons fw_devlink=on couldn't be set by default. Now that
> > > > > > > > >> cyclic
> > > > > > > > >> dependencies don't block probing, set fw_devlink=on by
> > > > > > > > >> default.
> > > > > > > > >>
> > > > > > > > >> Setting fw_devlink=on by default brings a bunch of benefits
> > > > > > > > >> (currently,
> > > > > > > > >> only for systems with device tree firmware):
> > > > > > > > >> * Significantly cuts down deferred probes.
> > > > > > > > >> * Device probe is effectively attempted in graph order.
> > > > > > > > >> * Makes it much easier to load drivers as modules without
> > > > > > > > >> having to
> > > > > > > > >> worry about functional dependencies between modules
> > > > > > > > >> (depmod is still
> > > > > > > > >> needed for symbol dependencies).
> > > > > > > > >>
> > > > > > > > >> If this patch prevents some devices from probing, it's very
> > > > > > > > >> likely due
> > > > > > > > >> to the system having one or more device drivers that
> > > > > > > > >> "probe"/set up a
> > > > > > > > >> device (DT node with compatible property) without creating a
> > > > > > > > >> struct
> > > > > > > > >> device for it. If we hit such cases, the device drivers
> > > > > > > > >> need to be
> > > > > > > > >> fixed so that they populate struct devices and probe them
> > > > > > > > >> like normal
> > > > > > > > >> device drivers so that the driver core is aware of the
> > > > > > > > >> devices and
> > > > > > > > >> their
> > > > > > > > >> status. See [1] for an example of such a case.
> > > > > > > > >>
> > > > > > > > >> [1] -
> > > > > > > > >> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> > > > > > > > >> Signed-off-by: Saravana Kannan
> > > > > > > > >
> > > > > > > > > Shimoda-san reported that next-20210111 and later fail to boot
> > > > > > > > > on Renesas R-Car Gen3 platforms. No output is seen, unless
> > > > > > > > > earlycon
> > > > > > > > > is enabled.
> > > > > > > > >
> > > > > > > > > I have bisected this to commit e590474768f1cc04 ("driver
> > > > > > > > > core: Set
> > > > > > > > > fw_devlink=on by default").
> > > > > > > >
> > > > > > > > There is a tentative patch from Saravana here[1], which works
> > > > > > > > around
> > > > > > > > some issues on my RK3399 platform, and it'd be interesting to
> > > > > > > > find
> > > > > > > > out whether that helps on your system.
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > >
> > > > > > > > M.
> > > > > > > >
> > > > > > > > [1]
> > > > > > > > https://lore.kernel.org/r/20210116011412.3211292-1-sarava...@google.com
> > > > > > >
> > > > > > > Thanks for the suggestion, but given no devices probe (incl. GPIO
> > > > > > > providers), I'm afraid it won't help. [testing] Indeed.
> > > > > > >
> > > > > > > With the debug prints in device_links_check_suppliers enabled, and
> > > > > > > some postprocessing, I get:
> > > > > > >
> > > > > > > 255 supplier e618.system-controller not ready
> > > > > > > 9 supplier fe99.iommu not ready
> > > > > > > 9 supplier fe98.iommu not ready
> > > > > > > 6 supplier febd.iommu not ready
> > > > > > > 6 supplier ec67.iommu not ready
> > > > > > > 3 supplier febe.iommu not ready
> > > > > > > 3 supplier e774.iommu not ready
> > > > > > > 3 supplier e674.iommu not ready
> > > > > > > 3 supplier e65ee000.usb-phy not ready
> > > > > > > 3 supplier e657.iommu not ready
> > > > > > > 3 supplier e6054000.gpio not ready
> > > > > > > 3 supplier e6053000.gpio not ready
> > > > > > >
> > > > > > > As everything is part of a PM Domain, the (lack of the) system
> > > > > > > controller
> > > > > > > must be the culprit. What's wrong with it? It is registered very
> > > > > > > early in
> > > > > > > the boot:
> > > > > > >
> > > > > > > [0.142096] rcar_sysc_pd_init:442:
> > > > > > > of_genpd_add_provider_onecell() returned 0
> > > > >
> > > > > > Looks like you found the important logs. Can you please enable all
> >
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi Saravana,
On Wed, Jan 20, 2021 at 10:40 AM Geert Uytterhoeven
wrote:
> On Tue, Jan 19, 2021 at 10:51 PM Saravana Kannan wrote:
> > On Tue, Jan 19, 2021 at 10:08 AM Saravana Kannan
> > wrote:
> > > On Tue, Jan 19, 2021 at 1:05 AM Geert Uytterhoeven
> > > wrote:
> > > > On Mon, Jan 18, 2021 at 10:19 PM Saravana Kannan
> > > > wrote:
> > > > > On Mon, Jan 18, 2021 at 11:16 AM Geert Uytterhoeven
> > > > > wrote:
> > > > > > On Mon, Jan 18, 2021 at 6:59 PM Marc Zyngier
> > > > > > wrote:
> > > > > > > On 2021-01-18 17:39, Geert Uytterhoeven wrote:
> > > > > > > > On Fri, Dec 18, 2020 at 4:34 AM Saravana Kannan
> > > > > > > >
> > > > > > > > wrote:
> > > > > > > >> Cyclic dependencies in some firmware was one of the last
> > > > > > > >> remaining
> > > > > > > >> reasons fw_devlink=on couldn't be set by default. Now that
> > > > > > > >> cyclic
> > > > > > > >> dependencies don't block probing, set fw_devlink=on by default.
> > > > > > > >>
> > > > > > > >> Setting fw_devlink=on by default brings a bunch of benefits
> > > > > > > >> (currently,
> > > > > > > >> only for systems with device tree firmware):
> > > > > > > >> * Significantly cuts down deferred probes.
> > > > > > > >> * Device probe is effectively attempted in graph order.
> > > > > > > >> * Makes it much easier to load drivers as modules without
> > > > > > > >> having to
> > > > > > > >> worry about functional dependencies between modules (depmod
> > > > > > > >> is still
> > > > > > > >> needed for symbol dependencies).
> > > > > > > >>
> > > > > > > >> If this patch prevents some devices from probing, it's very
> > > > > > > >> likely due
> > > > > > > >> to the system having one or more device drivers that
> > > > > > > >> "probe"/set up a
> > > > > > > >> device (DT node with compatible property) without creating a
> > > > > > > >> struct
> > > > > > > >> device for it. If we hit such cases, the device drivers need
> > > > > > > >> to be
> > > > > > > >> fixed so that they populate struct devices and probe them like
> > > > > > > >> normal
> > > > > > > >> device drivers so that the driver core is aware of the devices
> > > > > > > >> and
> > > > > > > >> their
> > > > > > > >> status. See [1] for an example of such a case.
> > > > > > > >>
> > > > > > > >> [1] -
> > > > > > > >> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> > > > > > > >> Signed-off-by: Saravana Kannan
> > > > > > > >
> > > > > > > > Shimoda-san reported that next-20210111 and later fail to boot
> > > > > > > > on Renesas R-Car Gen3 platforms. No output is seen, unless
> > > > > > > > earlycon
> > > > > > > > is enabled.
> > > > > > > >
> > > > > > > > I have bisected this to commit e590474768f1cc04 ("driver core:
> > > > > > > > Set
> > > > > > > > fw_devlink=on by default").
> > > > > > >
> > > > > > > There is a tentative patch from Saravana here[1], which works
> > > > > > > around
> > > > > > > some issues on my RK3399 platform, and it'd be interesting to find
> > > > > > > out whether that helps on your system.
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > M.
> > > > > > >
> > > > > > > [1]
> > > > > > > https://lore.kernel.org/r/20210116011412.3211292-1-sarava...@google.com
> > > > > >
> > > > > > Thanks for the suggestion, but given no devices probe (incl. GPIO
> > > > > > providers), I'm afraid it won't help. [testing] Indeed.
> > > > > >
> > > > > > With the debug prints in device_links_check_suppliers enabled, and
> > > > > > some postprocessing, I get:
> > > > > >
> > > > > > 255 supplier e618.system-controller not ready
> > > > > > 9 supplier fe99.iommu not ready
> > > > > > 9 supplier fe98.iommu not ready
> > > > > > 6 supplier febd.iommu not ready
> > > > > > 6 supplier ec67.iommu not ready
> > > > > > 3 supplier febe.iommu not ready
> > > > > > 3 supplier e774.iommu not ready
> > > > > > 3 supplier e674.iommu not ready
> > > > > > 3 supplier e65ee000.usb-phy not ready
> > > > > > 3 supplier e657.iommu not ready
> > > > > > 3 supplier e6054000.gpio not ready
> > > > > > 3 supplier e6053000.gpio not ready
> > > > > >
> > > > > > As everything is part of a PM Domain, the (lack of the) system
> > > > > > controller
> > > > > > must be the culprit. What's wrong with it? It is registered very
> > > > > > early in
> > > > > > the boot:
> > > > > >
> > > > > > [0.142096] rcar_sysc_pd_init:442:
> > > > > > of_genpd_add_provider_onecell() returned 0
> > > >
> > > > > Looks like you found the important logs. Can you please enable all
> > > > > these logs and send the early con logs as an attachment (so I don't
> > > > > need to deal with lines getting wrapped)?
> > > > > 1. The ones in device_links_check_suppliers()
> > > > > 2. The ones in device_link_add()
> > > > > 3. initcall_debug=1
> > > >
> > > > I have attached[*] the requested log.
>
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi Saravana,
On Tue, Jan 19, 2021 at 7:09 PM Saravana Kannan wrote:
> On Tue, Jan 19, 2021 at 1:05 AM Geert Uytterhoeven
> wrote:
> > BTW, you are aware IOMMUs and DMA controllers are optional?
> > I.e. device drivers with iommus and/or dmas DT properties where the
> > targets of these properties do not have a driver should still be probed,
> > eventually. But if the IOMMU or DMA drivers are present, they should be
> > probed first, so the device drivers can make use of them.
>
> Yeah, this is going to be a problem then. How is this handled in
> static kernels today? Do we just try to make sure the iommus driver
> probes the iommu device before the consumers? And then the consumers
> simply don't defer probe on failure to get iommu?
Iommus are handled by the iommu framework, not by the driver.
So the framework decides if/when it's OK to probe a device tied to an
iommu. Hence the consumers' drivers don't return -EPROBE_DEFER, the
framework takes care of that, before drivers' probe() functions are
called.
DMA is handled by consumer drivers, and driver-specific. Many consumer
drivers consider DMA optional, and fall back to PIO if getting the DMA
channel failed. Some drivers retry getting the DMA channel when the
device is used, and thus may start using DMA when the DMAC driver
appears and probes.
> I can make this work if modules are not enabled (needs some code
> changes), but it's not going to work when there are modules. There's
> no way to tell if an iommu module won't be loaded soon. Also, device
> links doing this behavior only for iommu/dma is probably not a good
> idea. So, whatever we do will have to be common behavior. :(
The iommu driver definitely needs to be built-in.
Modular DMAC drivers currently work with consumer drivers that
either consider DMA mandatory, or retry obtaining DMA channels.
> Also, can you try deleting "iommu" and "dma" parsing in
> of_supplier_bindings[] in driver/of/property.c and see if it helps?
> Then we'd know this is the reason for things not working in your case.
It also fails on another system without "iommus" properties:
182 supplier e618.system-controller not ready
18 supplier e6055400.gpio not ready
15 supplier e6055800.gpio not ready
15 supplier e6052000.gpio not ready
6 supplier e6055000.gpio not ready
The system controller is the culprit, and is a dependency for all
devices due to power-domains.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi Saravana,
On Tue, Jan 19, 2021 at 10:51 PM Saravana Kannan wrote:
> On Tue, Jan 19, 2021 at 10:08 AM Saravana Kannan wrote:
> > On Tue, Jan 19, 2021 at 1:05 AM Geert Uytterhoeven
> > wrote:
> > > On Mon, Jan 18, 2021 at 10:19 PM Saravana Kannan
> > > wrote:
> > > > On Mon, Jan 18, 2021 at 11:16 AM Geert Uytterhoeven
> > > > wrote:
> > > > > On Mon, Jan 18, 2021 at 6:59 PM Marc Zyngier wrote:
> > > > > > On 2021-01-18 17:39, Geert Uytterhoeven wrote:
> > > > > > > On Fri, Dec 18, 2020 at 4:34 AM Saravana Kannan
> > > > > > >
> > > > > > > wrote:
> > > > > > >> Cyclic dependencies in some firmware was one of the last
> > > > > > >> remaining
> > > > > > >> reasons fw_devlink=on couldn't be set by default. Now that cyclic
> > > > > > >> dependencies don't block probing, set fw_devlink=on by default.
> > > > > > >>
> > > > > > >> Setting fw_devlink=on by default brings a bunch of benefits
> > > > > > >> (currently,
> > > > > > >> only for systems with device tree firmware):
> > > > > > >> * Significantly cuts down deferred probes.
> > > > > > >> * Device probe is effectively attempted in graph order.
> > > > > > >> * Makes it much easier to load drivers as modules without having
> > > > > > >> to
> > > > > > >> worry about functional dependencies between modules (depmod is
> > > > > > >> still
> > > > > > >> needed for symbol dependencies).
> > > > > > >>
> > > > > > >> If this patch prevents some devices from probing, it's very
> > > > > > >> likely due
> > > > > > >> to the system having one or more device drivers that "probe"/set
> > > > > > >> up a
> > > > > > >> device (DT node with compatible property) without creating a
> > > > > > >> struct
> > > > > > >> device for it. If we hit such cases, the device drivers need to
> > > > > > >> be
> > > > > > >> fixed so that they populate struct devices and probe them like
> > > > > > >> normal
> > > > > > >> device drivers so that the driver core is aware of the devices
> > > > > > >> and
> > > > > > >> their
> > > > > > >> status. See [1] for an example of such a case.
> > > > > > >>
> > > > > > >> [1] -
> > > > > > >> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> > > > > > >> Signed-off-by: Saravana Kannan
> > > > > > >
> > > > > > > Shimoda-san reported that next-20210111 and later fail to boot
> > > > > > > on Renesas R-Car Gen3 platforms. No output is seen, unless
> > > > > > > earlycon
> > > > > > > is enabled.
> > > > > > >
> > > > > > > I have bisected this to commit e590474768f1cc04 ("driver core: Set
> > > > > > > fw_devlink=on by default").
> > > > > >
> > > > > > There is a tentative patch from Saravana here[1], which works around
> > > > > > some issues on my RK3399 platform, and it'd be interesting to find
> > > > > > out whether that helps on your system.
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > M.
> > > > > >
> > > > > > [1]
> > > > > > https://lore.kernel.org/r/20210116011412.3211292-1-sarava...@google.com
> > > > >
> > > > > Thanks for the suggestion, but given no devices probe (incl. GPIO
> > > > > providers), I'm afraid it won't help. [testing] Indeed.
> > > > >
> > > > > With the debug prints in device_links_check_suppliers enabled, and
> > > > > some postprocessing, I get:
> > > > >
> > > > > 255 supplier e618.system-controller not ready
> > > > > 9 supplier fe99.iommu not ready
> > > > > 9 supplier fe98.iommu not ready
> > > > > 6 supplier febd.iommu not ready
> > > > > 6 supplier ec67.iommu not ready
> > > > > 3 supplier febe.iommu not ready
> > > > > 3 supplier e774.iommu not ready
> > > > > 3 supplier e674.iommu not ready
> > > > > 3 supplier e65ee000.usb-phy not ready
> > > > > 3 supplier e657.iommu not ready
> > > > > 3 supplier e6054000.gpio not ready
> > > > > 3 supplier e6053000.gpio not ready
> > > > >
> > > > > As everything is part of a PM Domain, the (lack of the) system
> > > > > controller
> > > > > must be the culprit. What's wrong with it? It is registered very
> > > > > early in
> > > > > the boot:
> > > > >
> > > > > [0.142096] rcar_sysc_pd_init:442: of_genpd_add_provider_onecell()
> > > > > returned 0
> > >
> > > > Looks like you found the important logs. Can you please enable all
> > > > these logs and send the early con logs as an attachment (so I don't
> > > > need to deal with lines getting wrapped)?
> > > > 1. The ones in device_links_check_suppliers()
> > > > 2. The ones in device_link_add()
> > > > 3. initcall_debug=1
> > >
> > > I have attached[*] the requested log.
> > >
> > > > That should help us figure out what's going on. Also, what's the DT
> > > > that corresponds to one of the boards that see this issue?
> > >
> > > arch/arm64/boot/dts/renesas/r8a77951-salvator-xs.dts
> > >
> > > > Lastly, can you please pick up these 3 patches (some need clean up
> > > > before they merge) to make sure
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Tue, Jan 19, 2021 at 2:41 AM Michael Walle wrote: > > Am 2021-01-18 22:01, schrieb Saravana Kannan: > > On Sun, Jan 17, 2021 at 3:01 PM Michael Walle wrote: > >> > Cyclic dependencies in some firmware was one of the last remaining > >> > reasons fw_devlink=on couldn't be set by default. Now that cyclic > >> > dependencies don't block probing, set fw_devlink=on by default. > >> > > >> > Setting fw_devlink=on by default brings a bunch of benefits (currently, > >> > only for systems with device tree firmware): > >> > * Significantly cuts down deferred probes. > >> > * Device probe is effectively attempted in graph order. > >> > * Makes it much easier to load drivers as modules without having to > >> > worry about functional dependencies between modules (depmod is still > >> > needed for symbol dependencies). > >> > > >> > If this patch prevents some devices from probing, it's very likely due > >> > to the system having one or more device drivers that "probe"/set up a > >> > device (DT node with compatible property) without creating a struct > >> > device for it. If we hit such cases, the device drivers need to be > >> > fixed so that they populate struct devices and probe them like normal > >> > device drivers so that the driver core is aware of the devices and their > >> > status. See [1] for an example of such a case. > >> > > >> > [1] - > >> > https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/ > >> > Signed-off-by: Saravana Kannan > >> > >> This breaks (at least) probing of the PCIe controllers of my board. > >> The > >> driver in question is > >> drivers/pci/controller/dwc/pci-layerscape.c > >> I've also put the maintainers of this driver on CC. Looks like it uses > >> a > >> proper struct device. But it uses builtin_platform_driver_probe() and > >> apparently it waits for the iommu which uses module_platform_driver(). > >> Dunno if that will work together. > > > > Yeah, the builtin vs module doesn't matter. I've had fw_devlink work > > multiple times with the consumer driver being built in and the > > supplier actually loaded as a module. Making that work is one of the > > goals of fw_devlink. > > Ok. Hi Michael, My bad, I spoke too soon. I thought you were talking about builtin_ vs module_. My response is correct in that context. But the problem here is related to builtin_platform_driver_probe(). That macro expects the device (PCI) to be added and ready to probe by the time it's called. If not, it just gives up and frees the code. That's why it's not getting called after the first attempt. Can you please convert it into builtin_platform_driver()? It should be a pretty trivial change. -Saravana > > >> The board device tree can be found here: > >> arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts > >> > >> Attached is the log with enabled "probe deferral" messages enabled. > > > > I took a look at the logs. As you said, pci seems to be waiting on > > iommu, but it's not clear why the iommu didn't probe by then. Can you > > add initcall_debug=1 and enable the logs in device_link_add()? Btw, I > > realize one compromise on the logs is to send them as an attachment > > instead of inline. That way, it's still archived in the list, but I > > don't have to deal with log lines getting wrapped, etc. > > > > Thanks for reporting the issues. Also, could you try picking up all of > > these changes and giving it a shot. It's unlikely to help, but I want > > to rule out issues related to fixes in progress. > > > > https://lore.kernel.org/lkml/20210116011412.3211292-1-sarava...@google.com/ > > https://lore.kernel.org/lkml/20210115210159.3090203-1-sarava...@google.com/ > > https://lore.kernel.org/lkml/20201218210750.3455872-1-sarava...@google.com/ > > Did pick them up, the last one had a conflict due some superfluous > lines. > Maybe they got reordered in that arrray. > > Issue still persist. I've enabled the debug in device_link_add(), in > device_links_check_suppliers() and booted with initcall_debug. Please > see attached log. Lets see how that goes ;) > > [0.132687] calling ls_pcie_driver_init+0x0/0x38 @ 1 > [0.132762] platform 340.pcie: probe deferral - supplier > 500.iommu not ready > [0.132777] platform 350.pcie: probe deferral - supplier > 500.iommu not ready > [0.132818] initcall ls_pcie_driver_init+0x0/0x38 returned -19 after > 119 usecs > > After that, ls_pcie_driver_init() is never called again. > > -michael
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Tue, Jan 19, 2021 at 10:08 AM Saravana Kannan wrote:
>
> On Tue, Jan 19, 2021 at 1:05 AM Geert Uytterhoeven
> wrote:
> >
> > Hi Saravana,
> >
> > On Mon, Jan 18, 2021 at 10:19 PM Saravana Kannan
> > wrote:
> > > On Mon, Jan 18, 2021 at 11:16 AM Geert Uytterhoeven
> > > wrote:
> > > > On Mon, Jan 18, 2021 at 6:59 PM Marc Zyngier wrote:
> > > > > On 2021-01-18 17:39, Geert Uytterhoeven wrote:
> > > > > > On Fri, Dec 18, 2020 at 4:34 AM Saravana Kannan
> > > > > >
> > > > > > wrote:
> > > > > >> Cyclic dependencies in some firmware was one of the last remaining
> > > > > >> reasons fw_devlink=on couldn't be set by default. Now that cyclic
> > > > > >> dependencies don't block probing, set fw_devlink=on by default.
> > > > > >>
> > > > > >> Setting fw_devlink=on by default brings a bunch of benefits
> > > > > >> (currently,
> > > > > >> only for systems with device tree firmware):
> > > > > >> * Significantly cuts down deferred probes.
> > > > > >> * Device probe is effectively attempted in graph order.
> > > > > >> * Makes it much easier to load drivers as modules without having to
> > > > > >> worry about functional dependencies between modules (depmod is
> > > > > >> still
> > > > > >> needed for symbol dependencies).
> > > > > >>
> > > > > >> If this patch prevents some devices from probing, it's very likely
> > > > > >> due
> > > > > >> to the system having one or more device drivers that "probe"/set
> > > > > >> up a
> > > > > >> device (DT node with compatible property) without creating a struct
> > > > > >> device for it. If we hit such cases, the device drivers need to be
> > > > > >> fixed so that they populate struct devices and probe them like
> > > > > >> normal
> > > > > >> device drivers so that the driver core is aware of the devices and
> > > > > >> their
> > > > > >> status. See [1] for an example of such a case.
> > > > > >>
> > > > > >> [1] -
> > > > > >> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> > > > > >> Signed-off-by: Saravana Kannan
> > > > > >
> > > > > > Shimoda-san reported that next-20210111 and later fail to boot
> > > > > > on Renesas R-Car Gen3 platforms. No output is seen, unless earlycon
> > > > > > is enabled.
> > > > > >
> > > > > > I have bisected this to commit e590474768f1cc04 ("driver core: Set
> > > > > > fw_devlink=on by default").
> > > > >
> > > > > There is a tentative patch from Saravana here[1], which works around
> > > > > some issues on my RK3399 platform, and it'd be interesting to find
> > > > > out whether that helps on your system.
> > > > >
> > > > > Thanks,
> > > > >
> > > > > M.
> > > > >
> > > > > [1]
> > > > > https://lore.kernel.org/r/20210116011412.3211292-1-sarava...@google.com
> > > >
> > > > Thanks for the suggestion, but given no devices probe (incl. GPIO
> > > > providers), I'm afraid it won't help. [testing] Indeed.
> > > >
> > > > With the debug prints in device_links_check_suppliers enabled, and
> > > > some postprocessing, I get:
> > > >
> > > > 255 supplier e618.system-controller not ready
> > > > 9 supplier fe99.iommu not ready
> > > > 9 supplier fe98.iommu not ready
> > > > 6 supplier febd.iommu not ready
> > > > 6 supplier ec67.iommu not ready
> > > > 3 supplier febe.iommu not ready
> > > > 3 supplier e774.iommu not ready
> > > > 3 supplier e674.iommu not ready
> > > > 3 supplier e65ee000.usb-phy not ready
> > > > 3 supplier e657.iommu not ready
> > > > 3 supplier e6054000.gpio not ready
> > > > 3 supplier e6053000.gpio not ready
> > > >
> > > > As everything is part of a PM Domain, the (lack of the) system
> > > > controller
> > > > must be the culprit. What's wrong with it? It is registered very early
> > > > in
> > > > the boot:
> > > >
> > > > [0.142096] rcar_sysc_pd_init:442: of_genpd_add_provider_onecell()
> > > > returned 0
> >
> > > Looks like you found the important logs. Can you please enable all
> > > these logs and send the early con logs as an attachment (so I don't
> > > need to deal with lines getting wrapped)?
> > > 1. The ones in device_links_check_suppliers()
> > > 2. The ones in device_link_add()
> > > 3. initcall_debug=1
> >
> > I have attached[*] the requested log.
> >
> > > That should help us figure out what's going on. Also, what's the DT
> > > that corresponds to one of the boards that see this issue?
> >
> > arch/arm64/boot/dts/renesas/r8a77951-salvator-xs.dts
> >
> > > Lastly, can you please pick up these 3 patches (some need clean up
> > > before they merge) to make sure it's not an issue being worked on from
> > > other bug reports?
> > > https://lore.kernel.org/lkml/20210116011412.3211292-1-sarava...@google.com/
> > > https://lore.kernel.org/lkml/20210115210159.3090203-1-sarava...@google.com/
> > > https://lore.kernel.org/lkml/20201218210750.3455872-1-sarava...@google.com/
> > >
> > > I have a strong hunch the
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Tue, Jan 19, 2021 at 1:05 AM Geert Uytterhoeven wrote:
>
> Hi Saravana,
>
> On Mon, Jan 18, 2021 at 10:19 PM Saravana Kannan wrote:
> > On Mon, Jan 18, 2021 at 11:16 AM Geert Uytterhoeven
> > wrote:
> > > On Mon, Jan 18, 2021 at 6:59 PM Marc Zyngier wrote:
> > > > On 2021-01-18 17:39, Geert Uytterhoeven wrote:
> > > > > On Fri, Dec 18, 2020 at 4:34 AM Saravana Kannan
> > > > > wrote:
> > > > >> Cyclic dependencies in some firmware was one of the last remaining
> > > > >> reasons fw_devlink=on couldn't be set by default. Now that cyclic
> > > > >> dependencies don't block probing, set fw_devlink=on by default.
> > > > >>
> > > > >> Setting fw_devlink=on by default brings a bunch of benefits
> > > > >> (currently,
> > > > >> only for systems with device tree firmware):
> > > > >> * Significantly cuts down deferred probes.
> > > > >> * Device probe is effectively attempted in graph order.
> > > > >> * Makes it much easier to load drivers as modules without having to
> > > > >> worry about functional dependencies between modules (depmod is
> > > > >> still
> > > > >> needed for symbol dependencies).
> > > > >>
> > > > >> If this patch prevents some devices from probing, it's very likely
> > > > >> due
> > > > >> to the system having one or more device drivers that "probe"/set up a
> > > > >> device (DT node with compatible property) without creating a struct
> > > > >> device for it. If we hit such cases, the device drivers need to be
> > > > >> fixed so that they populate struct devices and probe them like normal
> > > > >> device drivers so that the driver core is aware of the devices and
> > > > >> their
> > > > >> status. See [1] for an example of such a case.
> > > > >>
> > > > >> [1] -
> > > > >> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> > > > >> Signed-off-by: Saravana Kannan
> > > > >
> > > > > Shimoda-san reported that next-20210111 and later fail to boot
> > > > > on Renesas R-Car Gen3 platforms. No output is seen, unless earlycon
> > > > > is enabled.
> > > > >
> > > > > I have bisected this to commit e590474768f1cc04 ("driver core: Set
> > > > > fw_devlink=on by default").
> > > >
> > > > There is a tentative patch from Saravana here[1], which works around
> > > > some issues on my RK3399 platform, and it'd be interesting to find
> > > > out whether that helps on your system.
> > > >
> > > > Thanks,
> > > >
> > > > M.
> > > >
> > > > [1]
> > > > https://lore.kernel.org/r/20210116011412.3211292-1-sarava...@google.com
> > >
> > > Thanks for the suggestion, but given no devices probe (incl. GPIO
> > > providers), I'm afraid it won't help. [testing] Indeed.
> > >
> > > With the debug prints in device_links_check_suppliers enabled, and
> > > some postprocessing, I get:
> > >
> > > 255 supplier e618.system-controller not ready
> > > 9 supplier fe99.iommu not ready
> > > 9 supplier fe98.iommu not ready
> > > 6 supplier febd.iommu not ready
> > > 6 supplier ec67.iommu not ready
> > > 3 supplier febe.iommu not ready
> > > 3 supplier e774.iommu not ready
> > > 3 supplier e674.iommu not ready
> > > 3 supplier e65ee000.usb-phy not ready
> > > 3 supplier e657.iommu not ready
> > > 3 supplier e6054000.gpio not ready
> > > 3 supplier e6053000.gpio not ready
> > >
> > > As everything is part of a PM Domain, the (lack of the) system controller
> > > must be the culprit. What's wrong with it? It is registered very early in
> > > the boot:
> > >
> > > [0.142096] rcar_sysc_pd_init:442: of_genpd_add_provider_onecell()
> > > returned 0
>
> > Looks like you found the important logs. Can you please enable all
> > these logs and send the early con logs as an attachment (so I don't
> > need to deal with lines getting wrapped)?
> > 1. The ones in device_links_check_suppliers()
> > 2. The ones in device_link_add()
> > 3. initcall_debug=1
>
> I have attached[*] the requested log.
>
> > That should help us figure out what's going on. Also, what's the DT
> > that corresponds to one of the boards that see this issue?
>
> arch/arm64/boot/dts/renesas/r8a77951-salvator-xs.dts
>
> > Lastly, can you please pick up these 3 patches (some need clean up
> > before they merge) to make sure it's not an issue being worked on from
> > other bug reports?
> > https://lore.kernel.org/lkml/20210116011412.3211292-1-sarava...@google.com/
> > https://lore.kernel.org/lkml/20210115210159.3090203-1-sarava...@google.com/
> > https://lore.kernel.org/lkml/20201218210750.3455872-1-sarava...@google.com/
> >
> > I have a strong hunch the 2nd one will fix your issues. fw_devlink can
> > handle cyclic dependencies now (it basically reverts to
> > fw_devlink=permissive mode for devices in the cycle), but it needs to
> > "see" all the dependencies to know there's a cycle. So want to make
> > sure it "sees" the "gpios" binding used all over some of the Renesas
> > DT
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Mon, Jan 18, 2021 at 11:16 AM Geert Uytterhoeven
wrote:
>
> Hi Marc,
>
> On Mon, Jan 18, 2021 at 6:59 PM Marc Zyngier wrote:
> > On 2021-01-18 17:39, Geert Uytterhoeven wrote:
> > > On Fri, Dec 18, 2020 at 4:34 AM Saravana Kannan
> > > wrote:
> > >> Cyclic dependencies in some firmware was one of the last remaining
> > >> reasons fw_devlink=on couldn't be set by default. Now that cyclic
> > >> dependencies don't block probing, set fw_devlink=on by default.
> > >>
> > >> Setting fw_devlink=on by default brings a bunch of benefits
> > >> (currently,
> > >> only for systems with device tree firmware):
> > >> * Significantly cuts down deferred probes.
> > >> * Device probe is effectively attempted in graph order.
> > >> * Makes it much easier to load drivers as modules without having to
> > >> worry about functional dependencies between modules (depmod is still
> > >> needed for symbol dependencies).
> > >>
> > >> If this patch prevents some devices from probing, it's very likely due
> > >> to the system having one or more device drivers that "probe"/set up a
> > >> device (DT node with compatible property) without creating a struct
> > >> device for it. If we hit such cases, the device drivers need to be
> > >> fixed so that they populate struct devices and probe them like normal
> > >> device drivers so that the driver core is aware of the devices and
> > >> their
> > >> status. See [1] for an example of such a case.
> > >>
> > >> [1] -
> > >> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> > >> Signed-off-by: Saravana Kannan
> > >
> > > Shimoda-san reported that next-20210111 and later fail to boot
> > > on Renesas R-Car Gen3 platforms. No output is seen, unless earlycon
> > > is enabled.
> > >
> > > I have bisected this to commit e590474768f1cc04 ("driver core: Set
> > > fw_devlink=on by default").
> >
> > There is a tentative patch from Saravana here[1], which works around
> > some issues on my RK3399 platform, and it'd be interesting to find
> > out whether that helps on your system.
> >
> > Thanks,
> >
> > M.
> >
> > [1]
> > https://lore.kernel.org/r/20210116011412.3211292-1-sarava...@google.com
>
> Thanks for the suggestion, but given no devices probe (incl. GPIO
> providers), I'm afraid it won't help. [testing] Indeed.
>
> With the debug prints in device_links_check_suppliers enabled, and
> some postprocessing, I get:
>
> 255 supplier e618.system-controller not ready
> 9 supplier fe99.iommu not ready
> 9 supplier fe98.iommu not ready
> 6 supplier febd.iommu not ready
> 6 supplier ec67.iommu not ready
> 3 supplier febe.iommu not ready
> 3 supplier e774.iommu not ready
> 3 supplier e674.iommu not ready
> 3 supplier e65ee000.usb-phy not ready
> 3 supplier e657.iommu not ready
> 3 supplier e6054000.gpio not ready
> 3 supplier e6053000.gpio not ready
>
> As everything is part of a PM Domain, the (lack of the) system controller
> must be the culprit. What's wrong with it? It is registered very early in
> the boot:
>
> [0.142096] rcar_sysc_pd_init:442: of_genpd_add_provider_onecell()
> returned 0
Hi Geert,
Thanks for reporting the issue.
Looks like you found the important logs. Can you please enable all
these logs and send the early con logs as an attachment (so I don't
need to deal with lines getting wrapped)?
1. The ones in device_links_check_suppliers()
2. The ones in device_link_add()
3. initcall_debug=1
That should help us figure out what's going on. Also, what's the DT
that corresponds to one of the boards that see this issue?
Lastly, can you please pick up these 3 patches (some need clean up
before they merge) to make sure it's not an issue being worked on from
other bug reports?
https://lore.kernel.org/lkml/20210116011412.3211292-1-sarava...@google.com/
https://lore.kernel.org/lkml/20210115210159.3090203-1-sarava...@google.com/
https://lore.kernel.org/lkml/20201218210750.3455872-1-sarava...@google.com/
I have a strong hunch the 2nd one will fix your issues. fw_devlink can
handle cyclic dependencies now (it basically reverts to
fw_devlink=permissive mode for devices in the cycle), but it needs to
"see" all the dependencies to know there's a cycle. So want to make
sure it "sees" the "gpios" binding used all over some of the Renesas
DT files.
Thanks,
Saravana
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Sun, Jan 17, 2021 at 3:01 PM Michael Walle wrote: > > Hi Saravana, again ;) Hi again! :) > > > Cyclic dependencies in some firmware was one of the last remaining > > reasons fw_devlink=on couldn't be set by default. Now that cyclic > > dependencies don't block probing, set fw_devlink=on by default. > > > > Setting fw_devlink=on by default brings a bunch of benefits (currently, > > only for systems with device tree firmware): > > * Significantly cuts down deferred probes. > > * Device probe is effectively attempted in graph order. > > * Makes it much easier to load drivers as modules without having to > > worry about functional dependencies between modules (depmod is still > > needed for symbol dependencies). > > > > If this patch prevents some devices from probing, it's very likely due > > to the system having one or more device drivers that "probe"/set up a > > device (DT node with compatible property) without creating a struct > > device for it. If we hit such cases, the device drivers need to be > > fixed so that they populate struct devices and probe them like normal > > device drivers so that the driver core is aware of the devices and their > > status. See [1] for an example of such a case. > > > > [1] - > > https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/ > > Signed-off-by: Saravana Kannan > > This breaks (at least) probing of the PCIe controllers of my board. The > driver in question is > drivers/pci/controller/dwc/pci-layerscape.c > I've also put the maintainers of this driver on CC. Looks like it uses a > proper struct device. But it uses builtin_platform_driver_probe() and > apparently it waits for the iommu which uses module_platform_driver(). > Dunno if that will work together. Yeah, the builtin vs module doesn't matter. I've had fw_devlink work multiple times with the consumer driver being built in and the supplier actually loaded as a module. Making that work is one of the goals of fw_devlink. > The board device tree can be found here: > arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts > > Attached is the log with enabled "probe deferral" messages enabled. I took a look at the logs. As you said, pci seems to be waiting on iommu, but it's not clear why the iommu didn't probe by then. Can you add initcall_debug=1 and enable the logs in device_link_add()? Btw, I realize one compromise on the logs is to send them as an attachment instead of inline. That way, it's still archived in the list, but I don't have to deal with log lines getting wrapped, etc. Thanks for reporting the issues. Also, could you try picking up all of these changes and giving it a shot. It's unlikely to help, but I want to rule out issues related to fixes in progress. https://lore.kernel.org/lkml/20210116011412.3211292-1-sarava...@google.com/ https://lore.kernel.org/lkml/20210115210159.3090203-1-sarava...@google.com/ https://lore.kernel.org/lkml/20201218210750.3455872-1-sarava...@google.com/ Thanks, Saravana
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On 2021-01-18 19:16, Geert Uytterhoeven wrote:
Hi Marc,
On Mon, Jan 18, 2021 at 6:59 PM Marc Zyngier wrote:
On 2021-01-18 17:39, Geert Uytterhoeven wrote:
> On Fri, Dec 18, 2020 at 4:34 AM Saravana Kannan
> wrote:
>> Cyclic dependencies in some firmware was one of the last remaining
>> reasons fw_devlink=on couldn't be set by default. Now that cyclic
>> dependencies don't block probing, set fw_devlink=on by default.
>>
>> Setting fw_devlink=on by default brings a bunch of benefits
>> (currently,
>> only for systems with device tree firmware):
>> * Significantly cuts down deferred probes.
>> * Device probe is effectively attempted in graph order.
>> * Makes it much easier to load drivers as modules without having to
>> worry about functional dependencies between modules (depmod is still
>> needed for symbol dependencies).
>>
>> If this patch prevents some devices from probing, it's very likely due
>> to the system having one or more device drivers that "probe"/set up a
>> device (DT node with compatible property) without creating a struct
>> device for it. If we hit such cases, the device drivers need to be
>> fixed so that they populate struct devices and probe them like normal
>> device drivers so that the driver core is aware of the devices and
>> their
>> status. See [1] for an example of such a case.
>>
>> [1] -
>>
https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
>> Signed-off-by: Saravana Kannan
>
> Shimoda-san reported that next-20210111 and later fail to boot
> on Renesas R-Car Gen3 platforms. No output is seen, unless earlycon
> is enabled.
>
> I have bisected this to commit e590474768f1cc04 ("driver core: Set
> fw_devlink=on by default").
There is a tentative patch from Saravana here[1], which works around
some issues on my RK3399 platform, and it'd be interesting to find
out whether that helps on your system.
Thanks,
M.
[1]
https://lore.kernel.org/r/20210116011412.3211292-1-sarava...@google.com
Thanks for the suggestion, but given no devices probe (incl. GPIO
providers), I'm afraid it won't help. [testing] Indeed.
With the debug prints in device_links_check_suppliers enabled, and
some postprocessing, I get:
255 supplier e618.system-controller not ready
9 supplier fe99.iommu not ready
9 supplier fe98.iommu not ready
6 supplier febd.iommu not ready
6 supplier ec67.iommu not ready
3 supplier febe.iommu not ready
3 supplier e774.iommu not ready
3 supplier e674.iommu not ready
3 supplier e65ee000.usb-phy not ready
3 supplier e657.iommu not ready
3 supplier e6054000.gpio not ready
3 supplier e6053000.gpio not ready
As everything is part of a PM Domain, the (lack of the) system
controller
must be the culprit. What's wrong with it? It is registered very early
in
the boot:
[0.142096] rcar_sysc_pd_init:442: of_genpd_add_provider_onecell()
returned 0
Yeah, this looks like the exact same problem. The devlink stuff assumes
that because there is a "compatible" property, there will be a driver
directly associated with the node containing this property.
If any other node has a reference to that first node, the dependency
will only get resolved if/when that first node is bound to a driver.
Trouble is, there are *tons* of code in the tree that invalidate
this heuristic, and for each occurrence of this we get another failure.
The patch I referred to papers over it by registering a dummy driver,
but that doesn't scale easily...
M.
--
Jazz is not dead. It just smells funny...
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi Marc,
On Mon, Jan 18, 2021 at 6:59 PM Marc Zyngier wrote:
> On 2021-01-18 17:39, Geert Uytterhoeven wrote:
> > On Fri, Dec 18, 2020 at 4:34 AM Saravana Kannan
> > wrote:
> >> Cyclic dependencies in some firmware was one of the last remaining
> >> reasons fw_devlink=on couldn't be set by default. Now that cyclic
> >> dependencies don't block probing, set fw_devlink=on by default.
> >>
> >> Setting fw_devlink=on by default brings a bunch of benefits
> >> (currently,
> >> only for systems with device tree firmware):
> >> * Significantly cuts down deferred probes.
> >> * Device probe is effectively attempted in graph order.
> >> * Makes it much easier to load drivers as modules without having to
> >> worry about functional dependencies between modules (depmod is still
> >> needed for symbol dependencies).
> >>
> >> If this patch prevents some devices from probing, it's very likely due
> >> to the system having one or more device drivers that "probe"/set up a
> >> device (DT node with compatible property) without creating a struct
> >> device for it. If we hit such cases, the device drivers need to be
> >> fixed so that they populate struct devices and probe them like normal
> >> device drivers so that the driver core is aware of the devices and
> >> their
> >> status. See [1] for an example of such a case.
> >>
> >> [1] -
> >> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> >> Signed-off-by: Saravana Kannan
> >
> > Shimoda-san reported that next-20210111 and later fail to boot
> > on Renesas R-Car Gen3 platforms. No output is seen, unless earlycon
> > is enabled.
> >
> > I have bisected this to commit e590474768f1cc04 ("driver core: Set
> > fw_devlink=on by default").
>
> There is a tentative patch from Saravana here[1], which works around
> some issues on my RK3399 platform, and it'd be interesting to find
> out whether that helps on your system.
>
> Thanks,
>
> M.
>
> [1]
> https://lore.kernel.org/r/20210116011412.3211292-1-sarava...@google.com
Thanks for the suggestion, but given no devices probe (incl. GPIO
providers), I'm afraid it won't help. [testing] Indeed.
With the debug prints in device_links_check_suppliers enabled, and
some postprocessing, I get:
255 supplier e618.system-controller not ready
9 supplier fe99.iommu not ready
9 supplier fe98.iommu not ready
6 supplier febd.iommu not ready
6 supplier ec67.iommu not ready
3 supplier febe.iommu not ready
3 supplier e774.iommu not ready
3 supplier e674.iommu not ready
3 supplier e65ee000.usb-phy not ready
3 supplier e657.iommu not ready
3 supplier e6054000.gpio not ready
3 supplier e6053000.gpio not ready
As everything is part of a PM Domain, the (lack of the) system controller
must be the culprit. What's wrong with it? It is registered very early in
the boot:
[0.142096] rcar_sysc_pd_init:442: of_genpd_add_provider_onecell() returned 0
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi Geert,
On 2021-01-18 17:39, Geert Uytterhoeven wrote:
Hi Saravana,
On Fri, Dec 18, 2020 at 4:34 AM Saravana Kannan
wrote:
Cyclic dependencies in some firmware was one of the last remaining
reasons fw_devlink=on couldn't be set by default. Now that cyclic
dependencies don't block probing, set fw_devlink=on by default.
Setting fw_devlink=on by default brings a bunch of benefits
(currently,
only for systems with device tree firmware):
* Significantly cuts down deferred probes.
* Device probe is effectively attempted in graph order.
* Makes it much easier to load drivers as modules without having to
worry about functional dependencies between modules (depmod is still
needed for symbol dependencies).
If this patch prevents some devices from probing, it's very likely due
to the system having one or more device drivers that "probe"/set up a
device (DT node with compatible property) without creating a struct
device for it. If we hit such cases, the device drivers need to be
fixed so that they populate struct devices and probe them like normal
device drivers so that the driver core is aware of the devices and
their
status. See [1] for an example of such a case.
[1] -
https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
Signed-off-by: Saravana Kannan
Shimoda-san reported that next-20210111 and later fail to boot
on Renesas R-Car Gen3 platforms. No output is seen, unless earlycon
is enabled.
I have bisected this to commit e590474768f1cc04 ("driver core: Set
fw_devlink=on by default").
There is a tentative patch from Saravana here[1], which works around
some issues on my RK3399 platform, and it'd be interesting to find
out whether that helps on your system.
Thanks,
M.
[1]
https://lore.kernel.org/r/20210116011412.3211292-1-sarava...@google.com
--
Jazz is not dead. It just smells funny...
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi Saravana,
On Wed, Jan 13, 2021 at 3:34 AM Saravana Kannan wrote:
> On Mon, Jan 11, 2021 at 11:11 PM Marek Szyprowski
> wrote:
> > On 11.01.2021 22:47, Saravana Kannan wrote:
> > > On Mon, Jan 11, 2021 at 6:18 AM Marek Szyprowski
> > > wrote:
> > >> On 11.01.2021 12:12, Marek Szyprowski wrote:
> > >>> On 18.12.2020 04:17, Saravana Kannan wrote:
> > Cyclic dependencies in some firmware was one of the last remaining
> > reasons fw_devlink=on couldn't be set by default. Now that cyclic
> > dependencies don't block probing, set fw_devlink=on by default.
> >
> > Setting fw_devlink=on by default brings a bunch of benefits (currently,
> > only for systems with device tree firmware):
> > * Significantly cuts down deferred probes.
> > * Device probe is effectively attempted in graph order.
> > * Makes it much easier to load drivers as modules without having to
> > worry about functional dependencies between modules (depmod is
> > still
> > needed for symbol dependencies).
> >
> > If this patch prevents some devices from probing, it's very likely due
> > to the system having one or more device drivers that "probe"/set up a
> > device (DT node with compatible property) without creating a struct
> > device for it. If we hit such cases, the device drivers need to be
> > fixed so that they populate struct devices and probe them like normal
> > device drivers so that the driver core is aware of the devices and
> > their
> > status. See [1] for an example of such a case.
> >
> > [1] -
> > https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> > Signed-off-by: Saravana Kannan
> > >>> This patch landed recently in linux next-20210111 as commit
> > >>> e590474768f1 ("driver core: Set fw_devlink=on by default"). Sadly it
> > >>> breaks Exynos IOMMU operation, what causes lots of devices being
> > >>> deferred and not probed at all. I've briefly checked and noticed that
> > >>> exynos_sysmmu_probe() is never called after this patch. This is really
> > >>> strange for me, as the SYSMMU controllers on Exynos platform are
> > >>> regular platform devices registered by the OF code. The driver code is
> > >>> here: drivers/iommu/exynos-iommu.c, example dts:
> > >>> arch/arm/boot/dts/exynos3250.dtsi (compatible =
> > >>> "samsung,exynos-sysmmu").
> > >> Okay, I found the source of this problem. It is caused by Exynos power
> > >> domain driver, which is not platform driver yet. I will post a patch,
> > >> which converts it to the platform driver.
> > > Thanks Marek! Hopefully the debug logs I added were sufficient to
> > > figure out the reason.
> >
> > Frankly, it took me a while to figure out that device core waits for the
> > power domain devices. Maybe it would be possible to add some more debug
> > messages or hints? Like the reason of the deferred probe in
> > /sys/kernel/debug/devices_deferred ?
>
> There's already a /sys/devices/...//waiting_for_supplier file
> that tells you if the device is waiting for a supplier device to be
> added. That file goes away once the device probes. If the file has 1,
> then it's waiting for the supplier device to be added (like your
> case). If it's 0, then the device is just waiting on one of the
> existing suppliers to probe. You can find the existing suppliers
> through /sys/devices/...//supplier:*/supplier. Also, flip
> these dev_dbg() to dev_info() if you need more details about deferred
> probing.
How are we supposed to check the contents of that file, if the system
doesn't even boot into userspace with a ramdisk? All hardware drivers
fail to probe. The only thing that works is "earlycon keep_bootcon",
and kernel output just stops after a while.
Thanks for your suggestions!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi Saravana,
On Fri, Dec 18, 2020 at 4:34 AM Saravana Kannan wrote:
> Cyclic dependencies in some firmware was one of the last remaining
> reasons fw_devlink=on couldn't be set by default. Now that cyclic
> dependencies don't block probing, set fw_devlink=on by default.
>
> Setting fw_devlink=on by default brings a bunch of benefits (currently,
> only for systems with device tree firmware):
> * Significantly cuts down deferred probes.
> * Device probe is effectively attempted in graph order.
> * Makes it much easier to load drivers as modules without having to
> worry about functional dependencies between modules (depmod is still
> needed for symbol dependencies).
>
> If this patch prevents some devices from probing, it's very likely due
> to the system having one or more device drivers that "probe"/set up a
> device (DT node with compatible property) without creating a struct
> device for it. If we hit such cases, the device drivers need to be
> fixed so that they populate struct devices and probe them like normal
> device drivers so that the driver core is aware of the devices and their
> status. See [1] for an example of such a case.
>
> [1] -
> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> Signed-off-by: Saravana Kannan
Shimoda-san reported that next-20210111 and later fail to boot
on Renesas R-Car Gen3 platforms. No output is seen, unless earlycon
is enabled.
I have bisected this to commit e590474768f1cc04 ("driver core: Set
fw_devlink=on by default").
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi Saravana, again ;) > Cyclic dependencies in some firmware was one of the last remaining > reasons fw_devlink=on couldn't be set by default. Now that cyclic > dependencies don't block probing, set fw_devlink=on by default. > > Setting fw_devlink=on by default brings a bunch of benefits (currently, > only for systems with device tree firmware): > * Significantly cuts down deferred probes. > * Device probe is effectively attempted in graph order. > * Makes it much easier to load drivers as modules without having to > worry about functional dependencies between modules (depmod is still > needed for symbol dependencies). > > If this patch prevents some devices from probing, it's very likely due > to the system having one or more device drivers that "probe"/set up a > device (DT node with compatible property) without creating a struct > device for it. If we hit such cases, the device drivers need to be > fixed so that they populate struct devices and probe them like normal > device drivers so that the driver core is aware of the devices and their > status. See [1] for an example of such a case. > > [1] - > https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/ > Signed-off-by: Saravana Kannan This breaks (at least) probing of the PCIe controllers of my board. The driver in question is drivers/pci/controller/dwc/pci-layerscape.c I've also put the maintainers of this driver on CC. Looks like it uses a proper struct device. But it uses builtin_platform_driver_probe() and apparently it waits for the iommu which uses module_platform_driver(). Dunno if that will work together. The board device tree can be found here: arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts Attached is the log with enabled "probe deferral" messages enabled. [0.00] Booting Linux on physical CPU 0x00 [0x410fd083] [0.00] Linux version 5.11.0-rc3-next-20210115-00013-g43ea1c90dcc8-dirty (mwalle@mwalle01) (aarch64-linux-gnu-gcc (Debian 8.3.0-2) 8.3.0, GNU ld (GNU Binutils for Debian) 2.31.1) #357 SMP PREEMPT Sun Jan 17 23:46:11 CET 2021 [0.00] Machine model: Kontron SMARC-sAL28 (Single PHY) on SMARC Eval 2.0 carrier [0.00] efi: UEFI not found. [0.00] NUMA: No NUMA configuration found [0.00] NUMA: Faking a node at [mem 0x8000-0x0020] [0.00] NUMA: NODE_DATA [mem 0x20ff7d9200-0x20ff7dafff] [0.00] Zone ranges: [0.00] DMA [mem 0x8000-0x] [0.00] DMA32empty [0.00] Normal [mem 0x0001-0x0020] [0.00] Movable zone start for each node [0.00] Early memory node ranges [0.00] node 0: [mem 0x8000-0x] [0.00] node 0: [mem 0x00208000-0x0020] [0.00] Initmem setup node 0 [mem 0x8000-0x0020] [0.00] On node 0 totalpages: 1048576 [0.00] DMA zone: 8192 pages used for memmap [0.00] DMA zone: 0 pages reserved [0.00] DMA zone: 524288 pages, LIFO batch:63 [0.00] Normal zone: 8192 pages used for memmap [0.00] Normal zone: 524288 pages, LIFO batch:63 [0.00] cma: Reserved 32 MiB at 0xfcc0 [0.00] percpu: Embedded 31 pages/cpu s89752 r8192 d29032 u126976 [0.00] pcpu-alloc: s89752 r8192 d29032 u126976 alloc=31*4096 [0.00] pcpu-alloc: [0] 0 [0] 1 [0.00] Detected PIPT I-cache on CPU0 [0.00] CPU features: detected: GIC system register CPU interface [0.00] CPU features: detected: Spectre-v3a [0.00] CPU features: detected: Spectre-v2 [0.00] CPU features: detected: Spectre-v4 [0.00] CPU features: detected: ARM errata 1165522, 1319367, or 1530923 [0.00] Built 1 zonelists, mobility grouping on. Total pages: 1032192 [0.00] Policy zone: Normal [0.00] Kernel command line: debug root=/dev/mmcblk0p2 rootwait [0.00] Dentry cache hash table entries: 524288 (order: 10, 4194304 bytes, linear) [0.00] Inode-cache hash table entries: 262144 (order: 9, 2097152 bytes, linear) [0.00] mem auto-init: stack:off, heap alloc:off, heap free:off [0.00] software IO TLB: mapped [mem 0xf8c0-0xfcc0] (64MB) [0.00] Memory: 3987204K/4194304K available (14592K kernel code, 2024K rwdata, 5776K rodata, 4736K init, 848K bss, 174332K reserved, 32768K cma-reserved) [0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=2, Nodes=1 [0.00] ftrace: allocating 51093 entries in 200 pages [0.00] ftrace: allocated 200 pages with 3 groups [0.00] rcu: Preemptible hierarchical RCU implementation. [0.00] rcu: RCU restricting CPUs from NR_CPUS=256 to nr_cpu_ids=2. [0.00] Trampoline variant of Tasks RCU enabled. [0.00] Rude variant of Tasks RCU
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Wed, Jan 13, 2021 at 11:36 PM Marek Szyprowski
wrote:
>
> Hi Saravana,
>
> On 13.01.2021 20:23, Saravana Kannan wrote:
> > On Tue, Jan 12, 2021 at 11:04 PM Marek Szyprowski
> > wrote:
> >> On 12.01.2021 21:51, Saravana Kannan wrote:
> >>> On Mon, Jan 11, 2021 at 11:11 PM Marek Szyprowski
> >>> wrote:
> On 11.01.2021 22:47, Saravana Kannan wrote:
> > On Mon, Jan 11, 2021 at 6:18 AM Marek Szyprowski
> > wrote:
> >> On 11.01.2021 12:12, Marek Szyprowski wrote:
> >>> On 18.12.2020 04:17, Saravana Kannan wrote:
> Cyclic dependencies in some firmware was one of the last remaining
> reasons fw_devlink=on couldn't be set by default. Now that cyclic
> dependencies don't block probing, set fw_devlink=on by default.
>
> Setting fw_devlink=on by default brings a bunch of benefits
> (currently,
> only for systems with device tree firmware):
> * Significantly cuts down deferred probes.
> * Device probe is effectively attempted in graph order.
> * Makes it much easier to load drivers as modules without having to
> worry about functional dependencies between modules (depmod is
> still
> needed for symbol dependencies).
>
> If this patch prevents some devices from probing, it's very likely
> due
> to the system having one or more device drivers that "probe"/set up a
> device (DT node with compatible property) without creating a struct
> device for it. If we hit such cases, the device drivers need to be
> fixed so that they populate struct devices and probe them like normal
> device drivers so that the driver core is aware of the devices and
> their
> status. See [1] for an example of such a case.
>
> [1] -
> https://protect2.fireeye.com/v1/url?k=68f5d8ba-376ee1f5-68f453f5-0cc47a30d446-324e64700545ab93=1=fb455b9e-c8c7-40d0-8e3c-d9d3713d519b=https%3A%2F%2Flore.kernel.org%2Flkml%2FCAGETcx9PiX%3D%3DmLxB9PO8Myyk6u2vhPVwTMsA5NkD-ywH5xhusw%40mail.gmail.com%2F
> Signed-off-by: Saravana Kannan
> >>> This patch landed recently in linux next-20210111 as commit
> >>> e590474768f1 ("driver core: Set fw_devlink=on by default"). Sadly it
> >>> breaks Exynos IOMMU operation, what causes lots of devices being
> >>> deferred and not probed at all. I've briefly checked and noticed that
> >>> exynos_sysmmu_probe() is never called after this patch. This is really
> >>> strange for me, as the SYSMMU controllers on Exynos platform are
> >>> regular platform devices registered by the OF code. The driver code is
> >>> here: drivers/iommu/exynos-iommu.c, example dts:
> >>> arch/arm/boot/dts/exynos3250.dtsi (compatible =
> >>> "samsung,exynos-sysmmu").
> >> Okay, I found the source of this problem. It is caused by Exynos power
> >> domain driver, which is not platform driver yet. I will post a patch,
> >> which converts it to the platform driver.
> > Thanks Marek! Hopefully the debug logs I added were sufficient to
> > figure out the reason.
> Frankly, it took me a while to figure out that device core waits for the
> power domain devices. Maybe it would be possible to add some more debug
> messages or hints? Like the reason of the deferred probe in
> /sys/kernel/debug/devices_deferred ?
> >>> There's already a /sys/devices/...//waiting_for_supplier file
> >>> that tells you if the device is waiting for a supplier device to be
> >>> added. That file goes away once the device probes. If the file has 1,
> >>> then it's waiting for the supplier device to be added (like your
> >>> case). If it's 0, then the device is just waiting on one of the
> >>> existing suppliers to probe. You can find the existing suppliers
> >>> through /sys/devices/...//supplier:*/supplier. Also, flip
> >>> these dev_dbg() to dev_info() if you need more details about deferred
> >>> probing.
> >> Frankly speaking I doubt that anyone will find those. Even experienced
> >> developer might need some time to figure it out.
> >>
> >> I expect that such information will be at least in the mentioned
> >> /sys/kernel/debug/devices_deferred file. We already have infrastructure
> >> for putting the deferred probe reason there, see dev_err_probe()
> >> function. Even such a simple change makes the debugging this issue much
> >> easier:
> >>
> >> diff --git a/drivers/base/core.c b/drivers/base/core.c
> >> index cd8e518fadd6..ceb5aed5a84c 100644
> >> --- a/drivers/base/core.c
> >> +++ b/drivers/base/core.c
> >> @@ -937,12 +937,13 @@ int device_links_check_suppliers(struct device *dev)
> >> mutex_lock(_link_lock);
> >> if (dev->fwnode && !list_empty(>fwnode->suppliers) &&
> >> !fw_devlink_is_permissive()) {
> >> - dev_dbg(dev, "probe deferral - wait for supplier %pfwP\n",
> >> +
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi Saravana,
On 13.01.2021 20:23, Saravana Kannan wrote:
> On Tue, Jan 12, 2021 at 11:04 PM Marek Szyprowski
> wrote:
>> On 12.01.2021 21:51, Saravana Kannan wrote:
>>> On Mon, Jan 11, 2021 at 11:11 PM Marek Szyprowski
>>> wrote:
On 11.01.2021 22:47, Saravana Kannan wrote:
> On Mon, Jan 11, 2021 at 6:18 AM Marek Szyprowski
> wrote:
>> On 11.01.2021 12:12, Marek Szyprowski wrote:
>>> On 18.12.2020 04:17, Saravana Kannan wrote:
Cyclic dependencies in some firmware was one of the last remaining
reasons fw_devlink=on couldn't be set by default. Now that cyclic
dependencies don't block probing, set fw_devlink=on by default.
Setting fw_devlink=on by default brings a bunch of benefits (currently,
only for systems with device tree firmware):
* Significantly cuts down deferred probes.
* Device probe is effectively attempted in graph order.
* Makes it much easier to load drivers as modules without having to
worry about functional dependencies between modules (depmod is
still
needed for symbol dependencies).
If this patch prevents some devices from probing, it's very likely due
to the system having one or more device drivers that "probe"/set up a
device (DT node with compatible property) without creating a struct
device for it. If we hit such cases, the device drivers need to be
fixed so that they populate struct devices and probe them like normal
device drivers so that the driver core is aware of the devices and
their
status. See [1] for an example of such a case.
[1] -
https://protect2.fireeye.com/v1/url?k=68f5d8ba-376ee1f5-68f453f5-0cc47a30d446-324e64700545ab93=1=fb455b9e-c8c7-40d0-8e3c-d9d3713d519b=https%3A%2F%2Flore.kernel.org%2Flkml%2FCAGETcx9PiX%3D%3DmLxB9PO8Myyk6u2vhPVwTMsA5NkD-ywH5xhusw%40mail.gmail.com%2F
Signed-off-by: Saravana Kannan
>>> This patch landed recently in linux next-20210111 as commit
>>> e590474768f1 ("driver core: Set fw_devlink=on by default"). Sadly it
>>> breaks Exynos IOMMU operation, what causes lots of devices being
>>> deferred and not probed at all. I've briefly checked and noticed that
>>> exynos_sysmmu_probe() is never called after this patch. This is really
>>> strange for me, as the SYSMMU controllers on Exynos platform are
>>> regular platform devices registered by the OF code. The driver code is
>>> here: drivers/iommu/exynos-iommu.c, example dts:
>>> arch/arm/boot/dts/exynos3250.dtsi (compatible =
>>> "samsung,exynos-sysmmu").
>> Okay, I found the source of this problem. It is caused by Exynos power
>> domain driver, which is not platform driver yet. I will post a patch,
>> which converts it to the platform driver.
> Thanks Marek! Hopefully the debug logs I added were sufficient to
> figure out the reason.
Frankly, it took me a while to figure out that device core waits for the
power domain devices. Maybe it would be possible to add some more debug
messages or hints? Like the reason of the deferred probe in
/sys/kernel/debug/devices_deferred ?
>>> There's already a /sys/devices/...//waiting_for_supplier file
>>> that tells you if the device is waiting for a supplier device to be
>>> added. That file goes away once the device probes. If the file has 1,
>>> then it's waiting for the supplier device to be added (like your
>>> case). If it's 0, then the device is just waiting on one of the
>>> existing suppliers to probe. You can find the existing suppliers
>>> through /sys/devices/...//supplier:*/supplier. Also, flip
>>> these dev_dbg() to dev_info() if you need more details about deferred
>>> probing.
>> Frankly speaking I doubt that anyone will find those. Even experienced
>> developer might need some time to figure it out.
>>
>> I expect that such information will be at least in the mentioned
>> /sys/kernel/debug/devices_deferred file. We already have infrastructure
>> for putting the deferred probe reason there, see dev_err_probe()
>> function. Even such a simple change makes the debugging this issue much
>> easier:
>>
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index cd8e518fadd6..ceb5aed5a84c 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -937,12 +937,13 @@ int device_links_check_suppliers(struct device *dev)
>> mutex_lock(_link_lock);
>> if (dev->fwnode && !list_empty(>fwnode->suppliers) &&
>> !fw_devlink_is_permissive()) {
>> - dev_dbg(dev, "probe deferral - wait for supplier %pfwP\n",
>> + ret = dev_err_probe(dev, -EPROBE_DEFER,
>> + "probe deferral - wait for supplier %pfwP\n",
>> list_first_entry(>fwnode->suppliers,
>> struct fwnode_link,
>>
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Tue, Jan 12, 2021 at 11:04 PM Marek Szyprowski
wrote:
>
> Hi Saravana,
>
> On 12.01.2021 21:51, Saravana Kannan wrote:
> > On Mon, Jan 11, 2021 at 11:11 PM Marek Szyprowski
> > wrote:
> >> On 11.01.2021 22:47, Saravana Kannan wrote:
> >>> On Mon, Jan 11, 2021 at 6:18 AM Marek Szyprowski
> >>> wrote:
> On 11.01.2021 12:12, Marek Szyprowski wrote:
> > On 18.12.2020 04:17, Saravana Kannan wrote:
> >> Cyclic dependencies in some firmware was one of the last remaining
> >> reasons fw_devlink=on couldn't be set by default. Now that cyclic
> >> dependencies don't block probing, set fw_devlink=on by default.
> >>
> >> Setting fw_devlink=on by default brings a bunch of benefits (currently,
> >> only for systems with device tree firmware):
> >> * Significantly cuts down deferred probes.
> >> * Device probe is effectively attempted in graph order.
> >> * Makes it much easier to load drivers as modules without having to
> >> worry about functional dependencies between modules (depmod is
> >> still
> >> needed for symbol dependencies).
> >>
> >> If this patch prevents some devices from probing, it's very likely due
> >> to the system having one or more device drivers that "probe"/set up a
> >> device (DT node with compatible property) without creating a struct
> >> device for it. If we hit such cases, the device drivers need to be
> >> fixed so that they populate struct devices and probe them like normal
> >> device drivers so that the driver core is aware of the devices and
> >> their
> >> status. See [1] for an example of such a case.
> >>
> >> [1] -
> >> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> >> Signed-off-by: Saravana Kannan
> > This patch landed recently in linux next-20210111 as commit
> > e590474768f1 ("driver core: Set fw_devlink=on by default"). Sadly it
> > breaks Exynos IOMMU operation, what causes lots of devices being
> > deferred and not probed at all. I've briefly checked and noticed that
> > exynos_sysmmu_probe() is never called after this patch. This is really
> > strange for me, as the SYSMMU controllers on Exynos platform are
> > regular platform devices registered by the OF code. The driver code is
> > here: drivers/iommu/exynos-iommu.c, example dts:
> > arch/arm/boot/dts/exynos3250.dtsi (compatible =
> > "samsung,exynos-sysmmu").
> Okay, I found the source of this problem. It is caused by Exynos power
> domain driver, which is not platform driver yet. I will post a patch,
> which converts it to the platform driver.
> >>> Thanks Marek! Hopefully the debug logs I added were sufficient to
> >>> figure out the reason.
> >> Frankly, it took me a while to figure out that device core waits for the
> >> power domain devices. Maybe it would be possible to add some more debug
> >> messages or hints? Like the reason of the deferred probe in
> >> /sys/kernel/debug/devices_deferred ?
> > There's already a /sys/devices/...//waiting_for_supplier file
> > that tells you if the device is waiting for a supplier device to be
> > added. That file goes away once the device probes. If the file has 1,
> > then it's waiting for the supplier device to be added (like your
> > case). If it's 0, then the device is just waiting on one of the
> > existing suppliers to probe. You can find the existing suppliers
> > through /sys/devices/...//supplier:*/supplier. Also, flip
> > these dev_dbg() to dev_info() if you need more details about deferred
> > probing.
>
> Frankly speaking I doubt that anyone will find those. Even experienced
> developer might need some time to figure it out.
>
> I expect that such information will be at least in the mentioned
> /sys/kernel/debug/devices_deferred file. We already have infrastructure
> for putting the deferred probe reason there, see dev_err_probe()
> function. Even such a simple change makes the debugging this issue much
> easier:
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index cd8e518fadd6..ceb5aed5a84c 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -937,12 +937,13 @@ int device_links_check_suppliers(struct device *dev)
> mutex_lock(_link_lock);
> if (dev->fwnode && !list_empty(>fwnode->suppliers) &&
> !fw_devlink_is_permissive()) {
> - dev_dbg(dev, "probe deferral - wait for supplier %pfwP\n",
> + ret = dev_err_probe(dev, -EPROBE_DEFER,
> + "probe deferral - wait for supplier %pfwP\n",
> list_first_entry(>fwnode->suppliers,
> struct fwnode_link,
> c_hook)->supplier);
> mutex_unlock(_link_lock);
> - return -EPROBE_DEFER;
> + return ret;
> }
> mutex_unlock(_link_lock);
>
> @@ -955,9 +956,9 @@ int
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi Saravana,
On 12.01.2021 21:51, Saravana Kannan wrote:
> On Mon, Jan 11, 2021 at 11:11 PM Marek Szyprowski
> wrote:
>> On 11.01.2021 22:47, Saravana Kannan wrote:
>>> On Mon, Jan 11, 2021 at 6:18 AM Marek Szyprowski
>>> wrote:
On 11.01.2021 12:12, Marek Szyprowski wrote:
> On 18.12.2020 04:17, Saravana Kannan wrote:
>> Cyclic dependencies in some firmware was one of the last remaining
>> reasons fw_devlink=on couldn't be set by default. Now that cyclic
>> dependencies don't block probing, set fw_devlink=on by default.
>>
>> Setting fw_devlink=on by default brings a bunch of benefits (currently,
>> only for systems with device tree firmware):
>> * Significantly cuts down deferred probes.
>> * Device probe is effectively attempted in graph order.
>> * Makes it much easier to load drivers as modules without having to
>> worry about functional dependencies between modules (depmod is still
>> needed for symbol dependencies).
>>
>> If this patch prevents some devices from probing, it's very likely due
>> to the system having one or more device drivers that "probe"/set up a
>> device (DT node with compatible property) without creating a struct
>> device for it. If we hit such cases, the device drivers need to be
>> fixed so that they populate struct devices and probe them like normal
>> device drivers so that the driver core is aware of the devices and their
>> status. See [1] for an example of such a case.
>>
>> [1] -
>> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
>> Signed-off-by: Saravana Kannan
> This patch landed recently in linux next-20210111 as commit
> e590474768f1 ("driver core: Set fw_devlink=on by default"). Sadly it
> breaks Exynos IOMMU operation, what causes lots of devices being
> deferred and not probed at all. I've briefly checked and noticed that
> exynos_sysmmu_probe() is never called after this patch. This is really
> strange for me, as the SYSMMU controllers on Exynos platform are
> regular platform devices registered by the OF code. The driver code is
> here: drivers/iommu/exynos-iommu.c, example dts:
> arch/arm/boot/dts/exynos3250.dtsi (compatible = "samsung,exynos-sysmmu").
Okay, I found the source of this problem. It is caused by Exynos power
domain driver, which is not platform driver yet. I will post a patch,
which converts it to the platform driver.
>>> Thanks Marek! Hopefully the debug logs I added were sufficient to
>>> figure out the reason.
>> Frankly, it took me a while to figure out that device core waits for the
>> power domain devices. Maybe it would be possible to add some more debug
>> messages or hints? Like the reason of the deferred probe in
>> /sys/kernel/debug/devices_deferred ?
> There's already a /sys/devices/...//waiting_for_supplier file
> that tells you if the device is waiting for a supplier device to be
> added. That file goes away once the device probes. If the file has 1,
> then it's waiting for the supplier device to be added (like your
> case). If it's 0, then the device is just waiting on one of the
> existing suppliers to probe. You can find the existing suppliers
> through /sys/devices/...//supplier:*/supplier. Also, flip
> these dev_dbg() to dev_info() if you need more details about deferred
> probing.
Frankly speaking I doubt that anyone will find those. Even experienced
developer might need some time to figure it out.
I expect that such information will be at least in the mentioned
/sys/kernel/debug/devices_deferred file. We already have infrastructure
for putting the deferred probe reason there, see dev_err_probe()
function. Even such a simple change makes the debugging this issue much
easier:
diff --git a/drivers/base/core.c b/drivers/base/core.c
index cd8e518fadd6..ceb5aed5a84c 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -937,12 +937,13 @@ int device_links_check_suppliers(struct device *dev)
mutex_lock(_link_lock);
if (dev->fwnode && !list_empty(>fwnode->suppliers) &&
!fw_devlink_is_permissive()) {
- dev_dbg(dev, "probe deferral - wait for supplier %pfwP\n",
+ ret = dev_err_probe(dev, -EPROBE_DEFER,
+ "probe deferral - wait for supplier %pfwP\n",
list_first_entry(>fwnode->suppliers,
struct fwnode_link,
c_hook)->supplier);
mutex_unlock(_link_lock);
- return -EPROBE_DEFER;
+ return ret;
}
mutex_unlock(_link_lock);
@@ -955,9 +956,9 @@ int device_links_check_suppliers(struct device *dev)
if (link->status != DL_STATE_AVAILABLE &&
!(link->flags & DL_FLAG_SYNC_STATE_ONLY)) {
device_links_missing_supplier(dev);
- dev_dbg(dev, "probe
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Mon, Jan 11, 2021 at 11:11 PM Marek Szyprowski
wrote:
>
> On 11.01.2021 22:47, Saravana Kannan wrote:
> > On Mon, Jan 11, 2021 at 6:18 AM Marek Szyprowski
> > wrote:
> >> On 11.01.2021 12:12, Marek Szyprowski wrote:
> >>> On 18.12.2020 04:17, Saravana Kannan wrote:
> Cyclic dependencies in some firmware was one of the last remaining
> reasons fw_devlink=on couldn't be set by default. Now that cyclic
> dependencies don't block probing, set fw_devlink=on by default.
>
> Setting fw_devlink=on by default brings a bunch of benefits (currently,
> only for systems with device tree firmware):
> * Significantly cuts down deferred probes.
> * Device probe is effectively attempted in graph order.
> * Makes it much easier to load drivers as modules without having to
> worry about functional dependencies between modules (depmod is still
> needed for symbol dependencies).
>
> If this patch prevents some devices from probing, it's very likely due
> to the system having one or more device drivers that "probe"/set up a
> device (DT node with compatible property) without creating a struct
> device for it. If we hit such cases, the device drivers need to be
> fixed so that they populate struct devices and probe them like normal
> device drivers so that the driver core is aware of the devices and their
> status. See [1] for an example of such a case.
>
> [1] -
> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> Signed-off-by: Saravana Kannan
> >>> This patch landed recently in linux next-20210111 as commit
> >>> e590474768f1 ("driver core: Set fw_devlink=on by default"). Sadly it
> >>> breaks Exynos IOMMU operation, what causes lots of devices being
> >>> deferred and not probed at all. I've briefly checked and noticed that
> >>> exynos_sysmmu_probe() is never called after this patch. This is really
> >>> strange for me, as the SYSMMU controllers on Exynos platform are
> >>> regular platform devices registered by the OF code. The driver code is
> >>> here: drivers/iommu/exynos-iommu.c, example dts:
> >>> arch/arm/boot/dts/exynos3250.dtsi (compatible = "samsung,exynos-sysmmu").
> >> Okay, I found the source of this problem. It is caused by Exynos power
> >> domain driver, which is not platform driver yet. I will post a patch,
> >> which converts it to the platform driver.
> > Thanks Marek! Hopefully the debug logs I added were sufficient to
> > figure out the reason.
>
> Frankly, it took me a while to figure out that device core waits for the
> power domain devices. Maybe it would be possible to add some more debug
> messages or hints? Like the reason of the deferred probe in
> /sys/kernel/debug/devices_deferred ?
There's already a /sys/devices/...//waiting_for_supplier file
that tells you if the device is waiting for a supplier device to be
added. That file goes away once the device probes. If the file has 1,
then it's waiting for the supplier device to be added (like your
case). If it's 0, then the device is just waiting on one of the
existing suppliers to probe. You can find the existing suppliers
through /sys/devices/...//supplier:*/supplier. Also, flip
these dev_dbg() to dev_info() if you need more details about deferred
probing.
https://lore.kernel.org/lkml/20201218031703.3053753-2-sarava...@google.com/
Hopefully this meets what you are looking for?
-Saravana
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On 11.01.2021 22:47, Saravana Kannan wrote:
> On Mon, Jan 11, 2021 at 6:18 AM Marek Szyprowski
> wrote:
>> On 11.01.2021 12:12, Marek Szyprowski wrote:
>>> On 18.12.2020 04:17, Saravana Kannan wrote:
Cyclic dependencies in some firmware was one of the last remaining
reasons fw_devlink=on couldn't be set by default. Now that cyclic
dependencies don't block probing, set fw_devlink=on by default.
Setting fw_devlink=on by default brings a bunch of benefits (currently,
only for systems with device tree firmware):
* Significantly cuts down deferred probes.
* Device probe is effectively attempted in graph order.
* Makes it much easier to load drivers as modules without having to
worry about functional dependencies between modules (depmod is still
needed for symbol dependencies).
If this patch prevents some devices from probing, it's very likely due
to the system having one or more device drivers that "probe"/set up a
device (DT node with compatible property) without creating a struct
device for it. If we hit such cases, the device drivers need to be
fixed so that they populate struct devices and probe them like normal
device drivers so that the driver core is aware of the devices and their
status. See [1] for an example of such a case.
[1] -
https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
Signed-off-by: Saravana Kannan
>>> This patch landed recently in linux next-20210111 as commit
>>> e590474768f1 ("driver core: Set fw_devlink=on by default"). Sadly it
>>> breaks Exynos IOMMU operation, what causes lots of devices being
>>> deferred and not probed at all. I've briefly checked and noticed that
>>> exynos_sysmmu_probe() is never called after this patch. This is really
>>> strange for me, as the SYSMMU controllers on Exynos platform are
>>> regular platform devices registered by the OF code. The driver code is
>>> here: drivers/iommu/exynos-iommu.c, example dts:
>>> arch/arm/boot/dts/exynos3250.dtsi (compatible = "samsung,exynos-sysmmu").
>> Okay, I found the source of this problem. It is caused by Exynos power
>> domain driver, which is not platform driver yet. I will post a patch,
>> which converts it to the platform driver.
> Thanks Marek! Hopefully the debug logs I added were sufficient to
> figure out the reason.
Frankly, it took me a while to figure out that device core waits for the
power domain devices. Maybe it would be possible to add some more debug
messages or hints? Like the reason of the deferred probe in
/sys/kernel/debug/devices_deferred ?
Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On Mon, Jan 11, 2021 at 6:18 AM Marek Szyprowski
wrote:
>
> On 11.01.2021 12:12, Marek Szyprowski wrote:
> > On 18.12.2020 04:17, Saravana Kannan wrote:
> >> Cyclic dependencies in some firmware was one of the last remaining
> >> reasons fw_devlink=on couldn't be set by default. Now that cyclic
> >> dependencies don't block probing, set fw_devlink=on by default.
> >>
> >> Setting fw_devlink=on by default brings a bunch of benefits (currently,
> >> only for systems with device tree firmware):
> >> * Significantly cuts down deferred probes.
> >> * Device probe is effectively attempted in graph order.
> >> * Makes it much easier to load drivers as modules without having to
> >>worry about functional dependencies between modules (depmod is still
> >>needed for symbol dependencies).
> >>
> >> If this patch prevents some devices from probing, it's very likely due
> >> to the system having one or more device drivers that "probe"/set up a
> >> device (DT node with compatible property) without creating a struct
> >> device for it. If we hit such cases, the device drivers need to be
> >> fixed so that they populate struct devices and probe them like normal
> >> device drivers so that the driver core is aware of the devices and their
> >> status. See [1] for an example of such a case.
> >>
> >> [1] -
> >> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> >> Signed-off-by: Saravana Kannan
> >
> > This patch landed recently in linux next-20210111 as commit
> > e590474768f1 ("driver core: Set fw_devlink=on by default"). Sadly it
> > breaks Exynos IOMMU operation, what causes lots of devices being
> > deferred and not probed at all. I've briefly checked and noticed that
> > exynos_sysmmu_probe() is never called after this patch. This is really
> > strange for me, as the SYSMMU controllers on Exynos platform are
> > regular platform devices registered by the OF code. The driver code is
> > here: drivers/iommu/exynos-iommu.c, example dts:
> > arch/arm/boot/dts/exynos3250.dtsi (compatible = "samsung,exynos-sysmmu").
>
> Okay, I found the source of this problem. It is caused by Exynos power
> domain driver, which is not platform driver yet. I will post a patch,
> which converts it to the platform driver.
Thanks Marek! Hopefully the debug logs I added were sufficient to
figure out the reason.
-Saravana
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
On 11.01.2021 12:12, Marek Szyprowski wrote:
> On 18.12.2020 04:17, Saravana Kannan wrote:
>> Cyclic dependencies in some firmware was one of the last remaining
>> reasons fw_devlink=on couldn't be set by default. Now that cyclic
>> dependencies don't block probing, set fw_devlink=on by default.
>>
>> Setting fw_devlink=on by default brings a bunch of benefits (currently,
>> only for systems with device tree firmware):
>> * Significantly cuts down deferred probes.
>> * Device probe is effectively attempted in graph order.
>> * Makes it much easier to load drivers as modules without having to
>> worry about functional dependencies between modules (depmod is still
>> needed for symbol dependencies).
>>
>> If this patch prevents some devices from probing, it's very likely due
>> to the system having one or more device drivers that "probe"/set up a
>> device (DT node with compatible property) without creating a struct
>> device for it. If we hit such cases, the device drivers need to be
>> fixed so that they populate struct devices and probe them like normal
>> device drivers so that the driver core is aware of the devices and their
>> status. See [1] for an example of such a case.
>>
>> [1] -
>> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
>> Signed-off-by: Saravana Kannan
>
> This patch landed recently in linux next-20210111 as commit
> e590474768f1 ("driver core: Set fw_devlink=on by default"). Sadly it
> breaks Exynos IOMMU operation, what causes lots of devices being
> deferred and not probed at all. I've briefly checked and noticed that
> exynos_sysmmu_probe() is never called after this patch. This is really
> strange for me, as the SYSMMU controllers on Exynos platform are
> regular platform devices registered by the OF code. The driver code is
> here: drivers/iommu/exynos-iommu.c, example dts:
> arch/arm/boot/dts/exynos3250.dtsi (compatible = "samsung,exynos-sysmmu").
Okay, I found the source of this problem. It is caused by Exynos power
domain driver, which is not platform driver yet. I will post a patch,
which converts it to the platform driver.
Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland
Re: [PATCH v1 5/5] driver core: Set fw_devlink=on by default
Hi Saravana,
On 18.12.2020 04:17, Saravana Kannan wrote:
> Cyclic dependencies in some firmware was one of the last remaining
> reasons fw_devlink=on couldn't be set by default. Now that cyclic
> dependencies don't block probing, set fw_devlink=on by default.
>
> Setting fw_devlink=on by default brings a bunch of benefits (currently,
> only for systems with device tree firmware):
> * Significantly cuts down deferred probes.
> * Device probe is effectively attempted in graph order.
> * Makes it much easier to load drivers as modules without having to
>worry about functional dependencies between modules (depmod is still
>needed for symbol dependencies).
>
> If this patch prevents some devices from probing, it's very likely due
> to the system having one or more device drivers that "probe"/set up a
> device (DT node with compatible property) without creating a struct
> device for it. If we hit such cases, the device drivers need to be
> fixed so that they populate struct devices and probe them like normal
> device drivers so that the driver core is aware of the devices and their
> status. See [1] for an example of such a case.
>
> [1] -
> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
> Signed-off-by: Saravana Kannan
This patch landed recently in linux next-20210111 as commit e590474768f1
("driver core: Set fw_devlink=on by default"). Sadly it breaks Exynos
IOMMU operation, what causes lots of devices being deferred and not
probed at all. I've briefly checked and noticed that
exynos_sysmmu_probe() is never called after this patch. This is really
strange for me, as the SYSMMU controllers on Exynos platform are regular
platform devices registered by the OF code. The driver code is here:
drivers/iommu/exynos-iommu.c, example dts:
arch/arm/boot/dts/exynos3250.dtsi (compatible = "samsung,exynos-sysmmu").
> ---
> drivers/base/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 4cc030361165..803bfa6eb823 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1457,7 +1457,7 @@ static void device_links_purge(struct device *dev)
> #define FW_DEVLINK_FLAGS_RPM(FW_DEVLINK_FLAGS_ON | \
>DL_FLAG_PM_RUNTIME)
>
> -static u32 fw_devlink_flags = FW_DEVLINK_FLAGS_PERMISSIVE;
> +static u32 fw_devlink_flags = FW_DEVLINK_FLAGS_ON;
> static int __init fw_devlink_setup(char *arg)
> {
> if (!arg)
Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland
[PATCH v1 5/5] driver core: Set fw_devlink=on by default
Cyclic dependencies in some firmware was one of the last remaining
reasons fw_devlink=on couldn't be set by default. Now that cyclic
dependencies don't block probing, set fw_devlink=on by default.
Setting fw_devlink=on by default brings a bunch of benefits (currently,
only for systems with device tree firmware):
* Significantly cuts down deferred probes.
* Device probe is effectively attempted in graph order.
* Makes it much easier to load drivers as modules without having to
worry about functional dependencies between modules (depmod is still
needed for symbol dependencies).
If this patch prevents some devices from probing, it's very likely due
to the system having one or more device drivers that "probe"/set up a
device (DT node with compatible property) without creating a struct
device for it. If we hit such cases, the device drivers need to be
fixed so that they populate struct devices and probe them like normal
device drivers so that the driver core is aware of the devices and their
status. See [1] for an example of such a case.
[1] -
https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com/
Signed-off-by: Saravana Kannan
---
drivers/base/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 4cc030361165..803bfa6eb823 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1457,7 +1457,7 @@ static void device_links_purge(struct device *dev)
#define FW_DEVLINK_FLAGS_RPM (FW_DEVLINK_FLAGS_ON | \
DL_FLAG_PM_RUNTIME)
-static u32 fw_devlink_flags = FW_DEVLINK_FLAGS_PERMISSIVE;
+static u32 fw_devlink_flags = FW_DEVLINK_FLAGS_ON;
static int __init fw_devlink_setup(char *arg)
{
if (!arg)
--
2.29.2.684.gfbc64c5ab5-goog
