Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arm64: dts: qcom: msm8916-lenovo-phab: add device #289

Draft
wants to merge 5 commits into
base: msm8916/6.2-rc5
Choose a base branch
from

Conversation

k1gen
Copy link

@k1gen k1gen commented Feb 23, 2023

Lenovo PHAB (PB1-750M) is a phablet using the
MSM8916 SoC released in late 2015.

Add a device tree with initial support for:

  • GPIO keys
  • Display & touchscreen
  • pm8916-vibrator
  • SDHCI (internal and external storage)
  • USB Device Mode
  • BMS & charging
  • UART (not tested)
  • WCNSS (WiFi/BT)
  • Modem (LTE works, calls & SMS not tested)
  • Regulators
  • Sound (WIP)
  • Accelerometer (WIP)

Signed-off-by: Oleksandr Knyshuk [email protected]

Lenovo PHAB (PB1-750M) is a phablet using the
MSM8916 SoC released in late 2015.

Add a device tree for with initial support for:

- GPIO keys
- pm8916-vibrator
- SDHCI (internal and external storage)
- USB Device Mode
- Charging
- UART (not tested)
- WCNSS (WiFi/BT)
- Modem (LTE works, calls & SMS not tested)
- Regulators
- Sound (WIP)
- Accelerometer (WIP)

Signed-off-by: Oleksandr Knyshuk <[email protected]>
Add jdi_1080p display panel with pwm and backlight
and FT5336 touchscreen with according pinctrls

Signed-off-by: Oleksandr Knyshuk <[email protected]>
arch/arm64/boot/dts/qcom/msm8916-lenovo-phab.dts Outdated Show resolved Hide resolved

&pm8916_bms {
monitored-battery = <&battery>;
//power-supplies = <&charger>;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem to be used or required.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It causes kernel panic, though it is the right way to use bms and charger together

};
};

&blsp_i2c5 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike display panels, touchscreen can be upstreamed with the driver in Torvald's tree, so you might want to split it from the display panel commit.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there an easy way to do it?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not known yet, I assume. You need to do this during git rebase.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do what, delete anyway?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Delete touchscreen parts, and commit with message blah blah
  2. Revert commit, and reword the revert with message arm64: dts: qcom: msm8916-lenovo-phab: add touchscreen
  3. Squash/fixup blah blah, and reword

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can probably reset the commit during the rebase, then split into two with git add --patch to only add some hunks (not sure if it would be split nicely though) and then squash the resulting touchscreen commit into the first one using another interactive rebase. Otherwise can probably interactive rebase -> edit first commit to add the TS and then fix the merge conflict when this commit applies.

}

static const struct of_device_id jdi_of_match[] = {
{ .compatible = "lenovo,phab-jdi_1080p" }, // FIXME
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{ .compatible = "lenovo,phab-jdi_1080p" }, // FIXME
{ .compatible = "lenovo,phab-jdi-1080p" }, // FIXME

or

Suggested change
{ .compatible = "lenovo,phab-jdi_1080p" }, // FIXME
{ .compatible = "lenovo,phab-jdi" }, // FIXME

You also need to fix it in msm8916-mainline/linux-panel-drivers#28 and generate the driver again.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can I use phab-panel?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only when you have defined lenovo,phab-panel in lk2nd and it selects from multiple panels.
msm8916-mainline/lk2nd@933fbc3

Anyway here is the naming rule, <vendor>,[device-]<panel_name>.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<vendor>,[device-]<panel_name>.

I see panel_name, not panel-name

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically it's meant to be dashes instead of underscores for compatible entries, if you know what I mean.

arch/arm64/boot/dts/qcom/msm8916-lenovo-phab.dts Outdated Show resolved Hide resolved
arch/arm64/boot/dts/qcom/msm8916-lenovo-phab.dts Outdated Show resolved Hide resolved
smb_int_default: smb-int-default-state {
pins = "gpio62";
function = "gpio";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

}

static const struct of_device_id jdi_of_match[] = {
{ .compatible = "lenovo,phab-jdi_1080p" }, // FIXME

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only when you have defined lenovo,phab-panel in lk2nd and it selects from multiple panels.
msm8916-mainline/lk2nd@933fbc3

Anyway here is the naming rule, <vendor>,[device-]<panel_name>.

};
};

&blsp_i2c5 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not known yet, I assume. You need to do this during git rebase.

@@ -18,6 +18,7 @@ dtb-$(CONFIG_ARCH_QCOM) += msm8916-asus-z010d.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-gplus-fl8005a.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-huawei-g7.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-huawei-y635.dtb
dtb-$(CONFIG_ARCH_QCOM) += msm8916-lenovo-phab.dtb
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit message: Please drop "not tested" :)

(you can do a reword with interactive rebase fyi)

Comment on lines +96 to +97
summit,enable-usb-charging;
summit,enable-mains-charging;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if you actually need both here... is "mains" required? and/or is there an example that uses mains that you have took this from? (I'd expect "mains" to mean "mains power" aka 220v or a barrel jack)

};
};

&blsp_i2c5 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can probably reset the commit during the rebase, then split into two with git add --patch to only add some hunks (not sure if it would be split nicely though) and then squash the resulting touchscreen commit into the first one using another interactive rebase. Otherwise can probably interactive rebase -> edit first commit to add the TS and then fix the merge conflict when this commit applies.

Comment on lines 178 to 179
qcom,platform-enable-gpio = <&msmgpio 109 GPIO_ACTIVE_HIGH>;
qcom,platform-reset-gpio = <&msmgpio 25 GPIO_ACTIVE_HIGH>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stephan-gh stephan-gh marked this pull request as draft April 7, 2023 18:42
Comment on lines +218 to +224
&pronto {
status = "okay";

iris {
compatible = "qcom,wcn3660b";
};
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A note about new changes since 6.3+: 3244442

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants