Page MenuHomeFreeBSD

mmel (Michal Meloun)
User

Projects

User Details

User Since
Feb 3 2015, 4:54 AM (380 w, 3 d)

Recent Activity

Feb 25 2022

mmel added a comment to D30560: sdhci: extend bus_dma_tag boundary to 64-bit space.

ping

Feb 25 2022, 7:59 AM

Feb 23 2022

mmel committed R10:01f8011736ff: iicbus: Add missing dependency for iicbus module. (authored by mmel).
iicbus: Add missing dependency for iicbus module.
Feb 23 2022, 3:31 PM
mmel added a comment to D34031: bus_if: Add a default implementation of get_property.

Hmm, I just did a simple test by adding a call to kdb_backtrace to ofw_bus_default_get_node and removing ofw_pci.c from the build.
Is this the scenario where you expected ENXIO to be returned from ofw_bus_get_node?
I'm not sure if I understand you correctly here.

Feb 23 2022, 8:45 AM

Feb 22 2022

mmel added a comment to D34031: bus_if: Add a default implementation of get_property.

This only applies to buses derived from simple/ofwbus, not to others (e.g. pci). For other device objects, the ofw_bus_get_node() function returns ENXIO (which may be a valid node ID). However, calling a class function on an object that is not derived from the given class should be considered an error.

Do you mean the case when kobj_lookup_method fails to find anything and we end up in kobj_error_method?
I don't think this is a possible scenario.
The default implementation - ofw_bus_default_get_node returns -1.
Note that BUS_GET_PROPERTY being resolved to simplebus_get_property implies that the bus was derived from ofw.
Because of that the OFW_BUS_GET_NODE will always have an implementation, possibly the default one.

Yes, I mean we end up in kobj_error_method. And that's a very real scenario. In newbus, the default method is only applied for the given class and its subclasses. *Not for all classes*

Feb 22 2022, 10:54 AM
mmel added a comment to D30560: sdhci: extend bus_dma_tag boundary to 64-bit space.

Unfortunately, it looks bad.
Up until SDHCI v3.0, SDMA is strictly 32-bit, only ADMA mode (which we don't support yet, it is optionally 64-bit). As you can see at https://cgit.freebsd.org/src/tree/sys/dev/sdhci/sdhci.c#n1977, the driver ends up writing only the 32-part of buffer physical address to the DMA base register.

Feb 22 2022, 6:29 AM
mmel added a comment to D34333: Support non-posted MMIO on arm64 with FDT.

Just a very minor notice . Wouldn't it be better to name the VM memory attribute by function, not by type? Different architectures may have different attribute mapping requirements for the PCI configuration space, and something like a global VM_MEMATTR_PCI_CONFIG (may be aliased to platform specific VM_MEMATTR_DEVICE_NP on arm64) gives us a much better chance of having cross-platform drivers without #ifdef hell.

Feb 22 2022, 5:54 AM

Feb 21 2022

mmel accepted D30079: Map arm64 pci config memory as non-posted.

Ahh right, thanks.

Feb 21 2022, 5:11 PM
mmel added a comment to D30079: Map arm64 pci config memory as non-posted.

There is also a tegra pcie driver that is common to arm and arm64 , so we should define VM_MEMATTR_DEVICE_NP for arm as well. But I'm not sure if we have the rest of the necessary infrastructure implemented on arm.

Feb 21 2022, 4:39 PM

Feb 20 2022

mmel committed R10:7504f32c2e59: ofw_iicbus: Use uint32_t for iic bus address argument, not int. (authored by mmel).
ofw_iicbus: Use uint32_t for iic bus address argument, not int.
Feb 20 2022, 1:49 PM
mmel committed R10:960ce3f7b2b0: Fix a534b50e245d on powerpc. (authored by mmel).
Fix a534b50e245d on powerpc.
Feb 20 2022, 1:49 PM
mmel committed R10:a534b50e245d: tegra: Improve the detection of the secondary function (RTC) of the MAX77620. (authored by mmel).
tegra: Improve the detection of the secondary function (RTC) of the MAX77620.
Feb 20 2022, 11:39 AM
mmel committed R10:1bd3e8ba6966: ofw_iicbus: Add method for manual setting of basic OFW parameters. (authored by mmel).
ofw_iicbus: Add method for manual setting of basic OFW parameters.
Feb 20 2022, 11:39 AM

Feb 18 2022

mmel added a comment to D34031: bus_if: Add a default implementation of get_property.

That's exactly how I initially did this, take a look at https://reviews.freebsd.org/D33457?id=100979#inline-208849.

I see and I think that original implementation was more correct.

Feb 18 2022, 4:26 PM
mmel added a comment to D34031: bus_if: Add a default implementation of get_property.

I don't think that's right. We can't expect that all indirect descendants of simplebus were instantiated by simplebus. Typically all enumerable buses (e.g. pci or multifunction devices represented by single FDT node) are not derived from simplebus. You cannot use ofw_bus_get_node(child) on those.
IMHO we can have a single generic implementation of <foo>_get_property for a given bus , but it should be explicitly defined in the device_methods structure for all appropriate drives.

Feb 18 2022, 2:20 PM

Feb 14 2022

mmel added a comment to D34027: sdhci_fsl_fdt: Fix tuning code.

Yes, I've applied all the patches. The problem is that the HS SD card (i.e. without debugging) also doesn't work and I don't know why.

Feb 14 2022, 1:03 PM
mmel added a comment to D34027: sdhci_fsl_fdt: Fix tuning code.

Due to problems with the D32706 I can't verify this on Honeycomb, plus I've never been able to have a working HW tuning. The current state is that this whole series of patches leaves Honeycomb with a non-functioning SD and eMMC. I currently have almost no free time to hack this problem, so everything is taking forever, sorry :(

Feb 14 2022, 12:20 PM

Feb 7 2022

mmel added a comment to D32706: sdhci_fsl_fdt: Provide more accurate clk calculation.

First of all, I apologize for being too harsh.
The problem is that this patch breaks both SD and eMMC on Honeycomb (LX2160). the eMMC is not detected at all, abd HSSSD card fires out with tons of CRC errors.
Unfortunately however this patch break SD and eMMC on Honezcomb (LX2160).
After further digging, I thing that fsl_sdhc_fdt_set_clock() is broken from day zero.
fsl_sdhc_fdt_set_clock() was supposed to be paired with fsl_sdhc_fdt_get_clock() to perform a conversion of differences between SDHCI and eSDHCl clock register format.

Feb 7 2022, 2:41 PM

Feb 5 2022

mmel added a comment to D32706: sdhci_fsl_fdt: Provide more accurate clk calculation.

Unfortunately this does not work at all and should be reverted.
eSDHC implements two versions of the clock divider, which are selected by the ESDHCCTL[CRS] bit.

Feb 5 2022, 9:17 AM

Jan 31 2022

mmel accepted D34028: sdhci_fsl_fdt: Apply errata for LX2160A.
Jan 31 2022, 9:42 AM
mmel accepted D33981: sdhci_fsl_fdt: Fix pulse width errata application.
Jan 31 2022, 9:41 AM

Jan 20 2022

mmel committed R10:d6529c0d12a2: pci_dw_mv: Don't enable unhandled interrupts. (authored by mmel).
pci_dw_mv: Don't enable unhandled interrupts.
Jan 20 2022, 10:37 AM
mmel committed R10:139afdb1721f: simple_mfd: switch to controllable locking for syscon provider. (authored by mmel).
simple_mfd: switch to controllable locking for syscon provider.
Jan 20 2022, 10:35 AM
mmel committed R10:8c12f9dea14f: mvebu_gpio: fix interrupt cause register configuration (authored by mw).
mvebu_gpio: fix interrupt cause register configuration
Jan 20 2022, 10:25 AM
mmel committed R10:40a0633862cd: mvebu_gpio: Fix settings of gpio pin direction. (authored by mmel).
mvebu_gpio: Fix settings of gpio pin direction.
Jan 20 2022, 10:25 AM
mmel committed R10:9ddb35ef1fd2: mvebu_gpio: Multiple fixes. (authored by mmel).
mvebu_gpio: Multiple fixes.
Jan 20 2022, 10:24 AM
mmel committed R10:f84b424eaf23: tegra/ahci: do not advertise enclosure management facility (authored by mmel).
tegra/ahci: do not advertise enclosure management facility
Jan 20 2022, 10:20 AM
mmel committed R10:7a647bb04500: tegra124: Implement new get_gate method for tegra124 clocks. (authored by mmel).
tegra124: Implement new get_gate method for tegra124 clocks.
Jan 20 2022, 10:19 AM
mmel committed R10:6f63b227bd2b: tegra210: Implement new get_gate method for tegra210 clocks. (authored by mmel).
tegra210: Implement new get_gate method for tegra210 clocks.
Jan 20 2022, 10:19 AM
mmel committed R10:f277be277de7: extres/clk: Add a method to detect the HW state of the clock gate. (authored by mmel).
extres/clk: Add a method to detect the HW state of the clock gate.
Jan 20 2022, 10:19 AM
mmel committed R10:3d4b9e5fa16a: extres/clk: Improve sysctl dump of clocks. (authored by mmel).
extres/clk: Improve sysctl dump of clocks.
Jan 20 2022, 10:18 AM
mmel committed R10:79631b49eeda: arm: Fix handling of undefined instruction aborts in THUMB2 mode. (authored by mmel).
arm: Fix handling of undefined instruction aborts in THUMB2 mode.
Jan 20 2022, 10:14 AM
mmel committed R10:a5e76683b2de: dwmmc: Calculate the maximum transaction length correctly. (authored by mmel).
dwmmc: Calculate the maximum transaction length correctly.
Jan 20 2022, 10:14 AM
mmel committed R10:8e3cc69a4e4a: booti: Enable loading the kernel image to any address aligned to 2 MB (authored by mmel).
booti: Enable loading the kernel image to any address aligned to 2 MB
Jan 20 2022, 10:14 AM
mmel committed R10:2ace1585b0f6: intrng: remove now redundant shadow variable. (authored by mmel).
intrng: remove now redundant shadow variable.
Jan 20 2022, 10:10 AM
mmel committed R10:a3ccd06dd965: intrng: Releasing interrupt source should clear interrupt table full state. (authored by mmel).
intrng: Releasing interrupt source should clear interrupt table full state.
Jan 20 2022, 10:10 AM
mmel committed R10:57dd17dd60e7: Fix error value returned by ofw_bus_gen_get_node(). (authored by mmel).
Fix error value returned by ofw_bus_gen_get_node().
Jan 20 2022, 10:07 AM

Jan 10 2022

mmel added inline comments to D33804: Remove the arm FIQ support.
Jan 10 2022, 2:49 PM

Jan 6 2022

mmel accepted D33761: libsoft: Remove.

LGTM

Jan 6 2022, 7:31 AM

Dec 30 2021

mmel accepted D33697: loader: tftp: Copy the first block into the cache.

Tried and tested - it fixes my previous problem, thanks.

Dec 30 2021, 11:07 AM

Dec 27 2021

mmel added a comment to D33445: clk: add initial frequency / gate config methods.

You know I'm "creative", especially when it comes to booting SBC.
The failing sequence is:

Dec 27 2021, 9:02 AM

Dec 26 2021

mmel accepted D33445: clk: add initial frequency / gate config methods.
Dec 26 2021, 12:05 PM

Dec 25 2021

mmel added a comment to D33410: loader: tftp: Add preload method.

Unfortunately thiss broke tftp kernel loading.
The tftp_open() function will prefetch the first block and leave tftpfile->currblock set to 1, so the first block is never loaded into the cache.
The trivial fix is to reset it back to 0, it works for me but I'm not sure if that's the right solution.

Dec 25 2021, 2:42 PM

Dec 24 2021

mmel committed R10:6e9119768dad: tegra/ahci: do not advertise enclosure management facility (authored by mmel).
tegra/ahci: do not advertise enclosure management facility
Dec 24 2021, 7:38 PM
mmel committed R10:be01656fa4cd: tegra124: Implement new get_gate method for tegra124 clocks. (authored by mmel).
tegra124: Implement new get_gate method for tegra124 clocks.
Dec 24 2021, 7:17 PM
mmel committed R10:7c0ec6638548: tegra210: Implement new get_gate method for tegra210 clocks. (authored by mmel).
tegra210: Implement new get_gate method for tegra210 clocks.
Dec 24 2021, 6:45 PM
mmel committed R10:1a74d77f8512: extres/clk: Add a method to detect the HW state of the clock gate. (authored by mmel).
extres/clk: Add a method to detect the HW state of the clock gate.
Dec 24 2021, 6:45 PM
mmel committed R10:72a2f3b5e28a: extres/clk: Improve sysctl dump of clocks. (authored by mmel).
extres/clk: Improve sysctl dump of clocks.
Dec 24 2021, 6:45 PM

Dec 23 2021

mmel requested changes to D33445: clk: add initial frequency / gate config methods.

Sorry for the strong language, please don't take it the wrong way.
clknode_init_freq_value() - this function is completely unnecessary, and only complicates an already complicated interface. Moreover, it is only used here as papering over a real problem in sysctl dump code. Ii really don't want that.

Dec 23 2021, 9:30 AM

Dec 10 2021

mmel accepted D33372: Fix set but not used warnings in arm64 rockchip.

LGTM

Dec 10 2021, 12:30 PM

Dec 9 2021

mmel added a comment to D33320: sdhci_fsl_fdt: Add support for HS200/HS400 modes .
ofw_bus_node_is_compatible(OF_finddevice("/"), "fsl,lx2160a")

does the trick (assuming that OF_finddevice("/") cannot fail)...

Dec 9 2021, 2:43 PM
mmel added a comment to D33320: sdhci_fsl_fdt: Add support for HS200/HS400 modes .

This breaks mmc on FDT based Honeycomb with following log:

Dec 9 2021, 11:05 AM
mmel added a comment to D33187: qoriq_therm: Fix sensor detection.

I see. But why you cannot use something like this (tested only on Honecomb) https://github.com/strejda/freebsd/commit/40eb737b2e9ca485daef8bf2effa96b053f847f9 ? Advantage of this code is that it can stay unchanged after thermal-zone driver was introduced..
I see. But why can't you use something like this (tested only on Honeycomb) https://github.com/strejda/freebsd/commit/40eb737b2e9ca485daef8bf2effa96b053f847f9 ? The advantage of this code is that it can remain unchanged after the thermal zone driver has been introduced.

Dec 9 2021, 10:55 AM
mmel accepted D33341: intrng: Use less confusing return value for intr_pic_add_handler.

LGTM

Dec 9 2021, 9:57 AM

Dec 8 2021

mmel added a comment to D33187: qoriq_therm: Fix sensor detection.

Sorry, I didn't realize we were talking about a different SoC.
For me, the only relevant source on the number of sensors is TRM, not the DT files.
The current code is ready for variable number of sensors, just fill a new struct tsensor array according to TRM and use that instead of default_sensors. Unfortunately, due to a strange habit in QorIQ DT we have to use compatible string of the root node itself instead of the compatible string of the thermal controller :( . See comment on (original) line 311.
Where do you see the problem?

Dec 8 2021, 5:29 PM

Dec 6 2021

mmel added a comment to D33187: qoriq_therm: Fix sensor detection.

I'm sorry, but I don't agree with this.
The temperature sensor is a different entity from a temperature zone which we can not mix. A given SOC always has a fixed number of temperature sensors but the number of temperature zones is determined by the design of the board/laptop/equipment. In addition, one temperature zone can be controlled by multiple temperature sensors (where each can have a different weight) and can control multiple cooling devices. A sensor may or may not be a member of a thermal zone and yet its value may be useful to the user (and thus accessible, for example, by sysctl(8)).
I have an initial implementation (almost ready to commit) of a temperature sensor framework and a very early implementation of thermal zones - both necessary for temperature controlled cooling. The problem is that I don't have the free time to finish them right now... So take what you want, or give me time over Christmas, I'll try to finish it to a committable state.

Dec 6 2021, 10:22 AM

Dec 1 2021

mmel added inline comments to D33140: [cpufreq] Support operating-mode-v2 tables with no voltages.
Dec 1 2021, 7:13 PM

Nov 2 2021

mmel accepted D32734: arm64: Don't rely on host readelf for u-boot booti image generation.

Thanks

Nov 2 2021, 11:58 AM
mmel committed R10:a670e1c13a52: arm: Fix handling of undefined instruction aborts in THUMB2 mode. (authored by mmel).
arm: Fix handling of undefined instruction aborts in THUMB2 mode.
Nov 2 2021, 10:35 AM

Oct 18 2021

mmel requested changes to D32387: dwmmc: switch to double-buffer structure.
Oct 18 2021, 2:15 PM
mmel added a comment to D32357: arm: add an interrupt rman to nexus.

on ARM (and on other intrng enabled systems) the interrupt resource does not represent the real interrupt - it is an arbitrarily assigned index that points to opaque interrupt mapping data collected by ofwbus - so multiple different interrupt resources can identify one actual interrupt or similar.

Oct 18 2021, 1:12 PM

Oct 8 2021

mmel committed R10:dfb736022285: dwmmc: Calculate the maximum transaction length correctly. (authored by mmel).
dwmmc: Calculate the maximum transaction length correctly.
Oct 8 2021, 7:22 AM
mmel closed D30387: Make the code consistent with the comments.
Oct 8 2021, 7:22 AM

Oct 1 2021

mmel added a comment to D31823: arm64: busdma bounce, handle nsegment==1 case better for load_phys().

I think adding more irregularity to an already ill-defined code behavior is not a good idea. Moreover, I think this is an obvious driver error - requesting a single segment for a buffer that can bounce is clear nonsense in which case the driver should allocate the buffer using bus_dmamem_alloc() or possibly pass the buffer aligned. Copying multiple pages back and forth doesn't sound like an optimal solution.

Oct 1 2021, 6:43 AM
mmel added inline comments to D32026: Add pmap_change_prot on arm64.
Oct 1 2021, 5:53 AM

Jul 28 2021

mmel committed R10:b07a6bd15a58: booti: Enable loading the kernel image to any address aligned to 2 MB (authored by mmel).
booti: Enable loading the kernel image to any address aligned to 2 MB
Jul 28 2021, 11:44 AM

Jul 23 2021

mmel accepted D31175: Support HWCAP/HWCAP2 for 32-bit ARM binaries..
Jul 23 2021, 8:27 PM

Jul 20 2021

mmel accepted D28803: arm/mv: Don't rely on firmware MSI mapping in ICU.

This looks perfect to me, many thanks.

Jul 20 2021, 7:29 PM

Jul 15 2021

mmel added inline comments to D31191: build: Rewrite arm_kernel_boothdr in Lua and add RISC-V support..
Jul 15 2021, 4:20 PM
mmel accepted D30474: arm64: Add per SoC family kernel config.
Jul 15 2021, 3:08 PM
mmel requested changes to D31191: build: Rewrite arm_kernel_boothdr in Lua and add RISC-V support..

In final commit, the original arm_kernel_boothdr.awk should be deleted. Otherwise look good to me

Jul 15 2021, 2:48 PM
mmel added a comment to D31175: Support HWCAP/HWCAP2 for 32-bit ARM binaries..

Otherwise looks good for me.,

Jul 15 2021, 8:06 AM

Jul 10 2021

mmel accepted D31030: pci_dw: Detect number of outbound regions automatically.

Tested on MACCHIATObin and HoneyComb LX2.

Jul 10 2021, 11:46 AM
mmel accepted D31029: pci_dw: Support modern "unroll" iATU mode.

Please, ignore my previous comment - I overlooked register name :(

Jul 10 2021, 11:46 AM
mmel accepted D31028: pci_dw: Support multiple memory windows.

Tested on MACCHIATObin and HoneyComb LX2 .

Jul 10 2021, 11:43 AM

Jul 8 2021

mmel accepted D31112: fan53555: attach to build and switch from syr827.
Jul 8 2021, 5:53 PM
mmel added a comment to D31103: syr827: add support for matching SYR828 as well (used by GPUs).

imho, only fan53555. And thanks for help.

Jul 8 2021, 1:13 PM
mmel added a comment to D31103: syr827: add support for matching SYR828 as well (used by GPUs).

If you can, do it yourself. My build environment is currently broken. I'm recovering from ZFS metadata corruption, plus ipfw (libalias) wants to divide by zero from time to time... :( Thanks.

Jul 8 2021, 12:44 PM
mmel committed R10:e88c3b1b02a6: intrng: remove now redundant shadow variable. (authored by mmel).
intrng: remove now redundant shadow variable.
Jul 8 2021, 6:59 AM
mmel committed R10:a49f208d94b8: intrng: Releasing interrupt source should clear interrupt table full state. (authored by mmel).
intrng: Releasing interrupt source should clear interrupt table full state.
Jul 8 2021, 6:45 AM
mmel added a comment to D31103: syr827: add support for matching SYR828 as well (used by GPUs).

We should switch to pmic/fan53555 driver which already supports it (along with other variants). I seem to have forgotten to do that. Can you test this, please?

Jul 8 2021, 6:06 AM

Jul 6 2021

mmel added a comment to D31030: pci_dw: Detect number of outbound regions automatically.

I need day or two to test this on my boards, but it looks OK for me.

Jul 6 2021, 6:52 AM
mmel added a comment to D31029: pci_dw: Support modern "unroll" iATU mode.

On correctly implemented systems, reading from an unimplemented device register will cause an exception (external asynchronous interrupt on arm64), on real systems it will return an arbitrary value.
Therefore, pci_dw_detect_atu_unroll() should only be called for modern core versions (which have the DW_IATU_VIEWPORT register implemented) - so we should check the core version first.
Otherwise looks good to me.

Jul 6 2021, 6:24 AM
mmel added a comment to D31069: rtld-elf/aarch64: Avoid checks for UND symbols when processing TPREL64.

For clarification, I meant this:

Jul 6 2021, 5:32 AM
mmel added a comment to D31069: rtld-elf/aarch64: Avoid checks for UND symbols when processing TPREL64.

However, the ELF definition also requires that the address of the undefined weak thread local symbol be translated to NULL. Additionally, I have seen code that relied on this. Unfortunately, because of aarch64's "unique" way of accessing TLS variables, I haven't found a way to implement this.
Wouldn't it be better to narrow down the cases where a message is printed, rather than allowing invalid and unreported behavior?

Jul 6 2021, 5:15 AM

Jun 24 2021

mmel committed R10:3eae4e106ac7: Fix error value returned by ofw_bus_gen_get_node(). (authored by mmel).
Fix error value returned by ofw_bus_gen_get_node().
Jun 24 2021, 10:06 AM

Jun 23 2021

mmel added a comment to D30761: ofw_bus_subr.c: Don't treat "0" as a valid OFW node.

nathan confirmed that ofw_bus_gen_get_node() should return -1 for non-ofw based device. i will commit fix asap (tomorrow).

Jun 23 2021, 3:34 PM
mmel added a comment to D30761: ofw_bus_subr.c: Don't treat "0" as a valid OFW node.

I'm working on this - but situation about return value becomes more clear now. Please see comment in ofw_if.m https://cgit.freebsd.org/src/tree/sys/dev/ofw/ofw_bus_if.m#n148 and andrew just pointed me to this commit https://cgit.freebsd.org/src/commit/?h=0d8d9edaaaca1

Jun 23 2021, 1:32 PM

Jun 19 2021

mmel accepted D30762: regulator: Add manpage this framework.

thx

Jun 19 2021, 8:38 AM
mmel accepted D30762: regulator: Add manpage this framework.

otherwise LGTM

Jun 19 2021, 8:22 AM

Jun 17 2021

mmel added a comment to D30761: ofw_bus_subr.c: Don't treat "0" as a valid OFW node.
In D30761#692573, @mmel wrote:

First, please don't take this as hating - I just think we've opened a Pandora's box full of mistakes... I've gotten caught up in the 0/-1 ambiguity for invalid phandle more than once, so I think it would be good to get that sorted out.

No worries, I agree that we should use only a single value to indicate a "NULL"/incorrent phandle.
Since it's a uint32_t I'd personally go with "0".

I understand but 0 is (probably only theoretically) valid pnode.

In D30761#692182, @mmel wrote:

Imho, this is just papering over real problem, it is obvious that ofw_bus_lookup_imap() should not be called for a device/bus that is not based on ofw.

ofw_bus_lookup_imap is called regardless of the ofw_pci patch.
pci_dw is a class 1 driver, which inherits devmethods from ofw_pcib. The latter implements pcib_route_interrupt using ofw_pcib_route_interrupt.
In other words ofw_bus_lookup_imap is called because the RC is ofw based.

Thats true, but something else is wrong in pcib_route_interrupt call-down hiearchy. This need slightly more time and deeper investigation...

I'm still trying to fully understand the real problem, but in the meantime I have a few questions:

  • Why ofw_bus_gen_get_node() returns a different value for the errored case than ofw_bus_default_get_node() ?

Frankly I have no idea. I haven't written any of those. I guess we wanted to differentiate between a case where there is no ofw support on the bus(-1) vs. where no node was found for a given device. (0)
Since phandle_t is an uint32_t type, assigning -1 to it is a bad idea imho.

After looking deeper into the dev/ofw source code, I noticed that the code only uses compare to -1 for error detection. So, I think ofw_bus_gen_get_node() is bad and should return -1 in case of error.

Well as mentioned above we should definitely return just one error value.
The problem is that changing the return value of ofw_bus_gen_get_node() could potentially break something else that relies on it returning 0.

That's true, but we have time to next release to fix all protentional problems.

Jun 17 2021, 6:38 PM
mmel added a comment to D30761: ofw_bus_subr.c: Don't treat "0" as a valid OFW node.

First, please don't take this as hating - I just think we've opened a Pandora's box full of mistakes... I've gotten caught up in the 0/-1 ambiguity for invalid phandle more than once, so I think it would be good to get that sorted out.

In D30761#692182, @mmel wrote:

Imho, this is just papering over real problem, it is obvious that ofw_bus_lookup_imap() should not be called for a device/bus that is not based on ofw.

ofw_bus_lookup_imap is called regardless of the ofw_pci patch.
pci_dw is a class 1 driver, which inherits devmethods from ofw_pcib. The latter implements pcib_route_interrupt using ofw_pcib_route_interrupt.
In other words ofw_bus_lookup_imap is called because the RC is ofw based.

Thats true, but something else is wrong in pcib_route_interrupt call-down hiearchy. This need slightly more time and deeper investigation...

Jun 17 2021, 2:25 PM

Jun 16 2021

mmel added a comment to D30761: ofw_bus_subr.c: Don't treat "0" as a valid OFW node.

Imho, this is just papering over real problem, it is obvious that ofw_bus_lookup_imap() should not be called for a device/bus that is not based on ofw.
I'm still trying to fully understand the real problem, but in the meantime I have a few questions:

  • Why ofw_bus_gen_get_node() returns a different value for the errored case than ofw_bus_default_get_node() ?
  • For all other buses (i2c, spi...) we have ofw and "native" variants -> why is PCI is special?
  • Why do we want to fake an enumerated pci bus (i.e. not ofw-based) as ofw-based?
  • Why do you think passing get_devinfo() to the parent is the correct default method, and why can it be used with all existing PCIE controllers?
  • How can ofw_pci work in case of more complex pcie topology (for example if the system has PCIe switches)?
Jun 16 2021, 1:38 PM

Jun 14 2021

mmel added a comment to D30181: Add ofw interface support to PCI.

yes, this commit is root cause .
Please see attached log:

Jun 14 2021, 1:09 PM
mmel added a comment to D30293: extres: regulator: Fix regulator_status for already enable regulators.

IMHO, this is wrong change. regulator_status() must report physical state of regulator - if given deice is powered or not. the mmc power sequencer should use similar technique as backlight. But seems like this kind of usage is common - so we can expand regulator framework with uncounted function -> something like regulator_on()/regulator_off()

Jun 14 2021, 12:22 PM
mmel added a comment to D30181: Add ofw interface support to PCI.

Unfortunately this broke (probably) all existing FDT enabled boards with enumerated (not FDT loaded) PCI(e) interfaces (in my case both mcbin and tegra). This driver probe() matches all PCI buses, and many things (driver, ofw code) depend on the fact that calling ofw_bus_get_node(dev) on a non-FDT installed device returns zero.

Jun 14 2021, 12:13 PM
mmel added a comment to D30387: Make the code consistent with the comments.

Sorry for long delay. Can you, please, also test "(IDMAC_MAX_SIZE * (IDMAC_DESC_SEGS - 1)) / MMC_SECTOR_SIZE"? Minus one should be sufficient and it passed all my tests.

Jun 14 2021, 11:42 AM

Mar 19 2021

mmel added inline comments to D29310: kern/intr: switch to allocating handles forward.
Mar 19 2021, 9:58 AM

Mar 18 2021

mmel requested changes to D29310: kern/intr: switch to allocating handles forward.
Mar 18 2021, 12:29 PM

Mar 7 2021

mmel committed R10:01c6d7918985: mvebu_gpio: Fix settings of gpio pin direction. (authored by mmel).
mvebu_gpio: Fix settings of gpio pin direction.
Mar 7 2021, 11:32 AM

Mar 4 2021

mmel committed R10:a5dce53b75d8: mvebu_gpio: Multiple fixes. (authored by mmel).
mvebu_gpio: Multiple fixes.
Mar 4 2021, 4:55 PM