Claude review: ARM: dts: ti: omap: espresso-common: Add common device tree for Samsung Galaxy Tab 2 series

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

This is the largest patch (744 lines) and has several issues:

**Bug - magnetometer interrupts property:**
```
magnetometer@2e {
    compatible = "yamaha,yas530";
    ...
    interrupts = <&gpio6 10 IRQ_TYPE_EDGE_RISING>;
};
```
The `interrupts` property takes plain interrupt specifiers, not phandle references. This should be either:
```
interrupt-parent = <&gpio6>;
interrupts = <10 IRQ_TYPE_EDGE_RISING>;
```
or:
```
interrupts-extended = <&gpio6 10 IRQ_TYPE_EDGE_RISING>;
```

**Issue - reserved-memory size hardcoded for 7" variant only:**
```
continuous_splash: framebuffer@bef00000{
    reg = <0xbef00000 (1024 * 600 * 4)>;
    no-map;
};
```
The size `1024 * 600 * 4` corresponds to the 7" panel (1024x600). The 10" panel is 1280x800, which requires `1280 * 800 * 4` bytes. This is in the **common** dtsi but the size is variant-specific. Either this should be moved to the per-variant dts files, or the size should be large enough for the largest panel, or the 10" variant should override it.

**Style - missing space before brace:**
```
continuous_splash: framebuffer@bef00000{
```
Should be `framebuffer@bef00000 {` (space before `{`).

**Typo in node name:**
```
ledir_pins: pimux-ledir-pins {
```
Should be `pinmux-ledir-pins` (missing `n` in `pinmux`).

**Inconsistent include syntax:**
```
#include "dt-bindings/gpio/gpio.h"
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/input/input.h>
```
The first include uses double quotes while the others use angle brackets. Should be consistent — prefer angle brackets (`<...>`) as used elsewhere in the kernel DTS files.

**Inconsistent hex prefix case in OMAP4_IOPAD:**
```
OMAP4_IOPAD(0X0D8, PIN_OUTPUT | PIN_OFF_OUTPUT_LOW | MUX_MODE3)
OMAP4_IOPAD(0X0D6, PIN_OUTPUT | PIN_OFF_OUTPUT_LOW | MUX_MODE1)
OMAP4_IOPAD(0X136, PIN_OUTPUT | MUX_MODE3)
```
These use uppercase `0X` prefix while all other entries use lowercase `0x`. Should be consistent (lowercase).

**Questionable interrupt flags on accelerometer:**
```
accelerometer@18 {
    ...
    interrupts = <25 (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_EDGE_RISING)>,
        <26 (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_EDGE_RISING)>;
};
```
Combining `IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_EDGE_RISING` (value 5) is unusual. Typically an interrupt is either level-triggered or edge-triggered, not both. Please verify this is correct for the BMA254 accelerometer.

**Unnecessary `#address-cells` in chosen:**
```
chosen {
    stdout-path = &uart3;
    #address-cells = <1>;
};
```
The `#address-cells` in the `chosen` node is unnecessary if there are no child nodes. Can be dropped.

**Minor - `ti,non-removable` in mmc2:**
```
&mmc2 {
    ...
    ti,non-removable;
    bus-width = <8>;
};
```
This is consistent with other OMAP DTS files that use the same property, so it's fine. (Though some newer DTS files use standard `non-removable`.)

---
Generated by Claude Code Patch Reviewer