> > + no-map;
> > + reg = <0xA0000000 0x200000>;
>
> If used for ramoops, then there should be a compatible = "ramoops"
> see Documentation/devicetree/bindings/reserved-memory/ramoops.yaml
> > + };
> > +
> > + continuous_splash: framebuffer@bef00000{
> > + reg = <0xbef00000 (1024 * 600 * 4)>;
> > + no-map;
> > + };
> > + };
> > +
> > + chosen {
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges;
>
> hmm, no bus here, so no need for address/size-cells, rather specify
> stdout-path, etc.
Will be dropping rampoops_region, and chosen nodes. They were used
initially for debugging, since we now have drm for display and other
means to get logs, these are not required.
> > + i2c-gpio-5 {
> > + compatible = "i2c-gpio";
> > + sda-gpios = <&gpio4 2 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > + scl-gpios = <&gpio4 3 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > + i2c-gpio,delay-us = <10>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> hmm, no pinmux here?
Cannot seem to find it in the muxset given in vendor kernel.
These are placeholders for now and hold other devices like smb136 charger,
stmpe811 adc etc. Drivers for which I need to upstream first.
https://github.com/MightyM17/linux_pvr/blob/testing/arch/arm/boot/dts/omap4-samsung-espresso7.dts#L10-L24
So for now is it better to drop them?
> > + reg_espresso_external: regulator-espresso-external {
> > + compatible = "regulator-fixed";
> > + regulator-name = "vmmc1";
> > + regulator-max-microvolt = <2800000>;
> > + regulator-min-microvolt = <2800000>;
> > + gpios = <&gpio2 2 GPIO_ACTIVE_HIGH>; /* GPIO_34 */
> > + regulator-always-on;
>
> hmm, we cannot turn sd card power off?
A mistake by me, had kept it always on to ensure it works in testing.
Fixed the error in next revision.
> > + power {
>
> button or key-power
Fixed.
> > + led-ir {
> > + compatible = "gpio-ir-tx";
> > + gpios = <&gpio2 27 GPIO_ACTIVE_HIGH>; /* GPIO_59 */
> pinmux?
Added one according to -
https://github.com/Unlegacy-Android/android_kernel_ti_omap4/blob/3.4/common/arch/arm/mach-omap2/board-espresso-irled.c#L303-L305
> > +&omap4_pmx_wkup {
>
> order node names alphabetically
Fixed.
> > + twl6030_wkup_pins: pinmux-twl6030-wkup-pins {
> > + pinctrl-single,pins = <
> > + OMAP4_IOPAD(0x54, PIN_OUTPUT | MUX_MODE3)
> > + /* fref_clk0_out.sys_drm_msecure */
> > + >;
> > + };
> > +
> This can come from twl6030_omap4.dtsi
Correct, included twl6030_omap4.dtsi and removed this.
> > +&omap4_pmx_core {
>
> order node names alphabetically
Fixed.
> you can simply include twl6030_omap4.dtsi describing recommended
> connection between omap4 SoCs and twl603X pmics.
Fixed.
> It might be an idea to use a dedicated wakeup irq instead of
> explicitely specifying WAKEUP_EN like you did for the uart.
> That counts for other occurances of WAKEUP_EN as well.
Could you point out to some examples having this change?
I have just followed how muxset mentioned it. I assume this can be
worked on later as well.
> generic node names:
> pmic@48
Changed.
> > + accelerometer@18 {
> > + compatible = "bosch,bma254";
> > + reg = <0x18>;
> > + vdd-supply = <&ldo4>;
> > + vddio-supply = <&ldo5>;
> > + interrupt-parent = <&gpio4>;
> > + interrupts = <25 (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_EDGE_RISING)>,
> > + <26 (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_EDGE_RISING)>;
>
> this looks odd, binding says IRQ_TYPE_EDGE_RISING. Why do you think you
> need both? After something is rising, it is high, so both seem not to
> make sense.
https://github.com/torvalds/linux/commit/5640fed3035e88c3ce1361e6fc93f4e72468f307
This was worked on before the above mentioned change, hence the confusion.
bma180 schema wants both the interrupts, I do not know why, but now it has
moved to the bma255 schema which makes more sense.
Fixed it according to new schema.
> + mount-matrix = "-1", "0", "0",
> + "0", "1", "0",
> + "0", "0", "1";
> hmm, checking twice, since I mixed up something earlier. This just
> inverts x values, so we are mirroring across y-z plane, that does not
> look like a rotation matrix, so it does not describe how it is mounted.
> Eg. the n900 has two -1 in there, that is a turn by 180 degree.
>
> Your mount-matrix would be achieved, by cutting the chip into ultra
> thin slices, sorting them upside down and glueing that together. I
> doubt somebody does that.
Went through the mount matrix docs multiple times. It seems fairly
straightforward for the accelerometer. being just a matrix that we can
multiply to get a desired result.
My intention is to flip the x values thus having a -1 in there.
What I do not understand is the logic of how you came to the conclusion
of "cutting the chip into ultra thin slices, sorting them upside down and
glueing".
The matrix seems correct and works as intended as well.
Best Regards,
Mithil
