Firstly, I must apologize. on my first response I didn't re-read the Rockchip TRM and instead relied on my own memory, which turned out to be a bad idea.

In D58212#1335515, @bz wrote:

I think the only thing I'd love to see is to tell users in the commit message how to apply this as it seems this needs to be done manually.

It would be significantly better to use predefined classes for fixed rate, fixed divider, gates or so .

At this stage, I do not want to introduce tables or hard-coded segment mappings
and/or limits for DMA memory allocation.
For flexibility, the primary interface should be based on functions. A default
implementation can always be added later if a sufficiently robust common case
is identified.

remove redundant empty line

update commit log

Rename function and fix cut&paste error

That's a very good and valid question.
Combining the fact that the 'dma-ranges' DT property (which this change targets) can describe multiple discontinuous ranges and that PCI(e) mappings further depend on transaction types quickly opens a very large Pandora's box. Add chained DMA tags and the IOMMU into the mix, and the problem becomes even more complex.
The honest answer is: I don't know how to handle this in full generality right now.
Today, it remains the driver's responsibility to work with CPU addresses. Modernising busdma would require a significant amount of work, and I'm not confident I have the energy or deep enough knowledge to tackle it all at once.
This minimal implementation is intended only to enable RPi5 support for now, nothing more yet.

All of these clock drivers share too much common code. Firstly, why don't you subclass these from the base class in mdtk_clk.c? If that's not sufficient, why don't you use a new base class that implements all this glue and subclass it?

I would much rather have PowerPC adapt the clock, reset and other frameworks. After all, PowerPC .dts files use them, so I don't see a single reason why we could not include them in the PowerPC build.
The assumption that FDT means that all these frameworks would make the world much simpler.

Aliasing SEGV_PKUERR and SEGV_MTEAERR breaks Qt6 (at least Qt6-base), as it has both in one switch. It seems Linux takes PKUERR as arch-independent.

LGTM

I'm not sure if anyone knows the exact, precise response. ARMv7 unwinding is pure mesh. All of my attempts to understand it ended with me turning off the computer and opening a bottle of good wine.
I think it's possible to write a backtrace that doesn't use it, but I also think it's used by the unwinder library, which is also used for backtraces.
My motivation was to be careful about security. If the executable implements real unwinder personalities, we should also use it for libsys

I understand that libsys might be the last frame.
However, this also means that the unwinder is supposed to call the function aeabi_unwind_cpp_pr0, found in the .ARM.exidx section of the corresponding object.
In your case, this is buildin_trap().
Is this right?

Sorry for my poorly formulated question.
The hidden but not weak aeabi_unwind_cpp_pr* symbols means that we don't expect or want to process exceptions generated in libsys because the unwinder always ends in an abort/trap. So why don't we simply compile libsys with -fno-exceptions?

Should not these symbols also be marked as weak? Hidden symbols are not 'overloaded' in the same library. kib, jrtc27 any comment about this, please?

It was should be closed with 62e1227219f2f79a2ed7a672149dc3d774d25dd4, sorry.

At first look only:

• • the '#ifndef MT7531' is clearly incompatible with GENERIC kernel.
• the defined FDT bindings between the NIC and the switch are not followed. Instead, a "brutal" hack (fdt_find_ethernet_prop_switch) is used.
• the eqos driver changes is improper. All of these are should be in attached the phy driver.

Use correct diff

Use correct diff

In D53891#1231630, @alc wrote:

In D53891#1231619, @mmel wrote:

Confirmed, this part is enough.

This makes sense to me. Suppose that the object has OBJ_ONEMAPPING set. On a memory-starved machine, we could have paged out all of the object's pages, so that the resident count would be zero. Then, if a hole is punched in a mapping to this object, then vm_map_entry_delete() will call vm_object_page_remove(), which will fail to release the swap space.

@mmel you have been testing on ARMv7, correct?

yes, and also on arm32 jail

Confirmed, this part is enough.

After another day of extensive testing, I can confirm that everything is working properly.

I apologize, I forgot about that. I thought I had already completed all phabricator tasks.

These are needed for cross-tree allocation of interrupts (and probably also pins). For example, in FDT, a device from another tree path, typically an I2C or SPI, may have its IRQ pin connected to GPIO.
A driver from another tree path can therefore use an GPIO based interrupt. These allocations are processed by the controller itself (due to FDT references), but by gpiobus.
@andrew is right, at least (de)activate_resource is necessary, but I'm not sure yet whether other resource list oriented methods are also needed.

I just found a similar problem with rockchip gpio. Implementing interrupt methods is not enough, we also need all the methods for resources. This led me to the idea that we could create a new class in which all these methods (interrupts and resources) would be implemented (by using bus_generic_<foo>), and then subclass all bus-like drivers from it. It's clean, simple, and saves a lot of lines...
@jhb, do you have any comments on this?

In D51932#1191682, @vexeduxr wrote:

In D51932#1191073, @mmel wrote:

We can introduce a new method, such as

METHOD  get_pin_list{
 device_t dev;
 device_t bus;
 uint32_t *pin_list;
};

which fills a pre-allocated array with the controller pin numbers, with a default implementation based on GPIO_PIN_MAX() (assuming it returns the number of controller pins - 1) and sequential numbering starting with pin number 0.
This way, we can obtain a list of pins for all 'standard' controllers without any changes to their code, but we retain the option to modify it for 'more creative' ones.

Consider this only as a first quick attempt to solve this puzzle.

That sounds good :)
Although I don't see why we would need the gpiobus dev.

Right, it's not necessary. Please ignore it.

We can introduce a new method, such as

METHOD  get_pin_list{
 device_t dev;
 device_t bus;
 uint32_t *pin_list;
};

which fills a pre-allocated array with the controller pin numbers, with a default implementation based on GPIO_PIN_MAX() (assuming it returns the number of controller pins - 1) and sequential numbering starting with pin number 0.
This way, we can obtain a list of pins for all 'standard' controllers without any changes to their code, but we retain the option to modify it for 'more creative' ones.

That seems fine to me. Unfortunately, I just found that config32 has num_pins as an argument, so it may not work on all 32 bits :( I didn't notice that before, sorry.

In D51931#1189574, @vexeduxr wrote:

In D51931#1189566, @mmel wrote:

Gpiobus should convert these virtual pin numbers to real ones (gpio controller) using IVARS and can assume absolutely nothing about the ordering/sequencing of these translated numbers.

Yeah I missed the ordering part, like the other review. I guess that means we can't check if they're all in the ivars before passing the call to the controller. All we can do in that case is make sure that devi->npins >= 32?

IMHO no, we should check if they are all in ivars, sequentially, starting with first_pin. See later.

The controller (i.e. using gpio ifc) access_32/configure_32 expects to work on 32/npins consecutive pin numbers. This means that the corresponding gpiobus methods must only work on slaves with 32/npins or more consecutive pin mappings.

Sorry, but this is making less and less sense to me.

Theoretically, every interrupt controller should implement all 3 of these functions.

Oups, my bad. I didn't realize that these are exposed by the ioct().
Only, Could you please check if the entire pin range
is within bound?

I like this step. But please give me one more day to re-analyze it.

pin_config_32 doesn't make sense to me at the moment. Ian created it at a time when we could only configure the direction of pins and only for specific hardware (if I remember correctly, iMX51). Now there is no chance that it could be implemented for current hardware and pin flags. Configuration settings are almost never atomic for a single pin, let alone for multiple pins.

Thanks for patience, I know how much frustrating can be these change requests based on "subjective" nuances .

I apologize. I should not write reviews when I am short on time.
I wanted to say that I fully agree with jhb. The gpiobus_attach_bus() function is too limited to be generally useful.
That is why I vote to replace it with the gpiobus_add_bus() function and explicit bus_attach_children().

Sorry for the delayed response, I've very busy in real life again.