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

General cleanup #701

Closed
wants to merge 6 commits into from
Closed

General cleanup #701

wants to merge 6 commits into from

Conversation

ix5
Copy link
Contributor

@ix5 ix5 commented Mar 6, 2020

Does this spark joy? I hope this sparks you for you!


hardware: ssc: Convert .mk to .bp

It's cleaner and faster to parse.

rootdir: Correct km4 init module name

The module is only shipping an init file, not an executable. Clear up this confusion my appending .rc to its module name.

rootdir: rdclean: Remove duplicate guard

The module will only be added for userdebug/eng builds in common.mk anyway.

rootdir: Make PLATFORM guards more logical

Having this makes little sense:

if (A && B && C)
if (B && C)
<add-stuff>
endif
<add-more-stuff>
endif

Rather, change it to:

if (B && C)
<add-stuff>
endif

if (A && B && C)
<add-more-stuff>
endif

rootdir: Drop msm8916 from guard for qmuxd.rc

This platform is not included in SODP Q.

rootdir: Convert .mk to .bp

Since soong doesn't know about templated module names, init.$(TARGET_DEVICE).rc and init.recovery.$(TARGET_DEVICE) stay as .mk.

Also, any module that is conditionally included on TARGET_* vars is left as-is.

rootdir: sscrdpcd.rc: Only for sdm845,sm8150

The service is not used by sdm660 and msm8998.


TODO

  • health: vintf_fragments for manifest
  • rootdir: Move guards to common-init

common-init.mk Outdated Show resolved Hide resolved
hardware/time-services/time_genoff.h Outdated Show resolved Hide resolved
Copy link
Contributor

@MarijnS95 MarijnS95 left a comment

Note that this breaks quite a few things. As you know product files (lowercase in our repos) are parsed first, then BoardConfig and descendants (CamelCase). Finally, module targets are read from Android.{bp,mk}, where these variables are available.

This PR moves all checking to common-init where the variables TARGET_NEEDS_AUDIOPD, WIFI_DRIVER_BUILT, TARGET_USES_TAD_V2 and TARGET_BOARD_PLATFORM are not defined yet. These will be defined later in the BoardConfig chain.

What do you reckon, should we clean up all platform/device files and move such simple, static declarations to the device product include chain? Some are there already anyway, while others live on in BoardConfig.

hardware/firmware/Android.mk Outdated Show resolved Hide resolved
@ix5
Copy link
Contributor Author

@ix5 ix5 commented Mar 7, 2020

This PR is dependent on a reorganization of the device makefiles, defining variables earlier in lowercase device.mk-inherited makfiles.

For now, the guards aren't touched. Will have to be addressed later.

Copy link
Contributor

@MarijnS95 MarijnS95 left a comment

Found some more breaking changes. But the final diff between vendor images for different devices start to align now, finally 😉

common-init.mk Outdated Show resolved Hide resolved
rootdir/Android.bp Outdated Show resolved Hide resolved
Copy link
Collaborator

@jerpelea jerpelea left a comment

do not remove the time services

Copy link
Collaborator

@jerpelea jerpelea left a comment

sscrpcd should not be added to sdm660 adn msm8998
this service is used only by the sdm845 and sm8150

@ix5
Copy link
Contributor Author

@ix5 ix5 commented Mar 9, 2020

sscrpcd should not be added to sdm660 adn msm8998
this service is used only by the sdm845 and sm8150

As it currently stands, sscrpd is included for msm8998 and later
ifneq ($(filter sdm660 msm8998 sdm845 sm8150,$(TARGET_BOARD_PLATFORM)),)
https://github.com/sonyxperiadev/device-sony-common/blob/master/rootdir/Android.mk#L320-L363

I can remove the init for 660 and 8998 though.

do not remove the time services

Ok

@jerpelea
Copy link
Collaborator

@jerpelea jerpelea commented Mar 10, 2020

sscrpcd should not be added to sdm660 adn msm8998
this service is used only by the sdm845 and sm8150

As it currently stands, sscrpd is included for msm8998 and later
ifneq ($(filter sdm660 msm8998 sdm845 sm8150,$(TARGET_BOARD_PLATFORM)),)
https://github.com/sonyxperiadev/device-sony-common/blob/master/rootdir/Android.mk#L320-L363

I can remove the init for 660 and 8998 though.

do not remove the time services

Ok

please do the blobs are not prezent on those devices

ix5 added a commit to ix5/device-sony-common that referenced this issue May 23, 2020
The service is not used by sdm660 and msm8998.

See:
sonyxperiadev#701 (review)
@ix5
Copy link
Contributor Author

@ix5 ix5 commented May 23, 2020

Pushed with less breaking changes

@ix5
Copy link
Contributor Author

@ix5 ix5 commented May 26, 2020

This pulls in #734 as well

@ix5 ix5 changed the title [WIP/DNM] General cleanup General cleanup May 26, 2020
@ix5 ix5 marked this pull request as ready for review May 26, 2020
@ix5 ix5 force-pushed the general-cleanup branch 2 times, most recently from baaae9b to 71fd337 Compare May 26, 2020
@MarijnS95
Copy link
Contributor

@MarijnS95 MarijnS95 commented May 26, 2020

@ix5 Can you pull in #735 too?

MarijnS95 pushed a commit to MarijnS95/device-sony-common that referenced this issue May 27, 2020
The service is not used by sdm660 and msm8998.

See:
sonyxperiadev#701 (review)
ix5 added 5 commits May 27, 2020
It's cleaner and faster to parse.
The module is only shipping an init file, not an executable.
Clear up this confusion my appending .rc to its module name.
The module will only be added for userdebug/eng builds in
common.mk anyway.
Having this makes little sense:
```
if (A && B && C)
if (B && C)
<add-stuff>
endif
<add-more-stuff>
endif
```

Rather, change it to:
```
if (B && C)
<add-stuff>
endif

if (A && B && C)
<add-more-stuff>
endif
```

...to improve readability.
This platform is not included in SODP Q.
Since soong doesn't know about templated module names,
`init.$(TARGET_DEVICE).rc` and
`init.recovery.$(TARGET_DEVICE)` stay as .mk.

Also, any module that is conditionally included on
`TARGET_*` vars is left as-is.
@ix5
Copy link
Contributor Author

@ix5 ix5 commented May 27, 2020

Rebased on top of latest changes

@jerpelea
Copy link
Collaborator

@jerpelea jerpelea commented May 28, 2020

cherry-picked

@ix5 ix5 closed this May 28, 2020
@ix5 ix5 deleted the general-cleanup branch May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants