ext: st: move ST HAL to its own repo #16044

nashif · 2019-05-09T17:51:08Z

Move ST HAL out of the tree and use zephyr module under
https://github.com/zephyrproject-rtos/hal_st instead.

Also includes #16039

Adds https://github.com/zephyrproject-rtos/hal_qmsi as a module managed by west. Signed-off-by: Anas Nashif <[email protected]>

Move ST HAL out of the tree and use zephyr module under https://github.com/zephyrproject-rtos/hal_st instead. Signed-off-by: Anas Nashif <[email protected]>

erwango · 2019-05-10T07:54:41Z

@nashif, I totally understand the need for this change, but inside ext/hal/st/lib, there are some libs that are needed to work with ST MEMS sensors.
These sensors could be used on any platform, so if we move them into hal_st, one will need to fetch whole hal_st to get 2 files to get his sensor working.
Can we think about keeping them into zephyr main tree (for instance under ext/st/lib)?

-3,888,367 lines

This is impressive!

nashif · 2019-05-10T10:26:25Z

Can we think about keeping them into zephyr main tree (for instance under ext/st/lib)?

we could also put them in their own module. Would they actually make sense on their own without the ST hal?
Btw, the hal modules is something you only get once and just update later, so I do not see why is this a problem.

erwango · 2019-05-10T11:42:24Z

we could also put them in their own module.

@avisconti, What is your opinion on this? What about a st_mems project which would include current ext/hal/st/libs and the new ST MEMS HAL you're pushing in #14111.
So instead of hal_st, we'd have hal_stm32cube and hal_stmems, or hal_stsensor

Would they actually make sense on their own without the ST hal?

These are totally independent products, so it definitely makes sense. One can use a ST MEMS sensor on any hw.

Btw, the hal modules is something you only get once and just update later, so I do not see why is this a problem.

Not a problem to me for sure, but I'm thinking about other people that'd need to pull the 10 stm32cube packages to compile one ST sensor driver on a non STM32 product. I can already hear them complaining.

erwango · 2019-05-10T12:18:30Z

@nashif, one question, is there a change related to this one that will update west.yml and set a default revision for each hal_xxx in default manifest for zephyr main repo?

nashif · 2019-05-10T12:31:47Z

@nashif, one question, is there a change related to this one that will update west.yml and set a default revision for each hal_xxx in default manifest for zephyr main repo?

yes. it is in the PR

erwango · 2019-05-10T12:34:40Z

yes. it is in the PR

Ok, I can see there a .yml file in the change, but github won't let me see it. Will see when pulling the change then.

avisconti · 2019-05-10T12:48:58Z

we could also put them in their own module.

@avisconti, What is your opinion on this? What about a st_mems project which would include current ext/hal/st/libs and the new ST MEMS HAL you're pushing in #14111.
So instead of hal_st, we'd have hal_stm32cube and hal_stmems, or hal_stsensor

Yes, #14111 is impacted. Not sure now what is the right thing to do. I mean whether to get it merged now or wait until the multi repository model approach is consolidated and then push it in the new separate repo. How long would it take to do this transition?

Would they actually make sense on their own without the ST hal?

These are totally independent products, so it definitely makes sense. One can use a ST MEMS sensor on any hw.

Correct! They are used to control different sensor chips which may be attached thru I2C/SPI to different MCUs. The ST sensors can be motion, environmental, proximity, microphones.
BTW, also #15566 is impacted (@overheat, you should take a look here as well)

Btw, the hal modules is something you only get once and just update later, so I do not see why is this a problem.

Not a problem to me for sure, but I'm thinking about other people that'd need to pull the 10 stm32cube packages to compile one ST sensor driver on a non STM32 product. I can already hear them complaining.

Maybe you are right Erwan, but I'm a little bit worried about the fact that we may have too many modules here. Currently I can count 4 different modules: stm32, stmems, lib/microphone, lib/imaging.
My understanding of the multi-repo model approach is that in any case the user is forced to download the source code. Then in building phase of course only the code used is compiled. But all the source should be there. Am I wrong?
If so maybe we can have a single module ST with all the code there.

EDIT::
I see now that the proposal is to only have two modules:

hal_stm32cube
hal_stsensor

In the latter module we may mix stmemsc (ext/hal/st/stmemsc), microphone (ext/hal/st/lib/audio) and imaging (ext/hal/st/lib/sensor). This may also work.

nashif · 2019-05-10T12:55:53Z

But all the source should be there. Am I wrong?

no, you are correct. This is not downloaded for every build, pretty much the same as we have it right now, just not under zephyr/ext/...

nashif · 2019-05-10T13:07:31Z

but I'm a little bit worried about the fact that we may have too many modules here.

too many modules is not a problem. Giving people the flexibility to pull just what they need for their products is a huge advantage. If I do not need the ST HAL and just need the stmems library, I should not be forced to pull everything (Users should be able to customize the west.yml file for their needs and applications and just pull and maintain the modules they use in their code)

erwango · 2019-05-10T13:31:19Z

@nashif, as part of this change you'd need to update CODEOWNERS as well.
Talking about that, what is impact on CODEOWNERS?
Should we get one CODEOWNERS per module?

avisconti · 2019-05-10T13:34:29Z

but I'm a little bit worried about the fact that we may have too many modules here.

too many modules is not a problem. Giving people the flexibility to pull just what they need for their products is a huge advantage. If I do not need the ST HAL and just need the stmems library, I should not be forced to pull everything (Users should be able to customize the west.yml file for their needs and applications and just pull and maintain the modules they use in their code)

I think you and Erwan are both right: if a user is not interested into stm32cube stuff because is using a different MCU, then it should not pull that code.

So, to finalize the discussion I think that we may have two separate modules as suggested by Erwan: hal_stm32cube and hal_stsensors.

Inside the hal_stsensors repo I would organize following HALs:

stmemsc ([RFC] ST sensor drivers migration to STdC #14111)
audio (/ext/hal/st/lib/audio, no need for 'lib' extra directory level)
proximity (/ext/hal/st/lib/sensor, no need for 'lib' extra directory level and proximity is a better name).

erwango · 2019-05-10T14:39:36Z

@avisconti , I'm fine with this.
@nashif, is that ok for you too?

nashif · 2019-05-10T14:55:31Z

@nashif, is that ok for you too?

yes, works for me.

erwango · 2019-05-14T15:00:49Z

@nashif, do you expect us to do some preparation work for what has been discussed above?

avisconti · 2019-05-15T08:30:34Z

@nashif, do you expect us to do some preparation work for what has been discussed above?

Maybe we should prepare hal_stm32cube and hal_stsensors repos under
https://github.com/zephyrproject-rtos/

erwango · 2019-05-16T11:07:33Z

Before going further, we need to elaborate a bit the proposal:
I see currently 2 additional needs for these that are not addressed with what we discussed:
-Non sensor related, non STM32 specific, ST libs (cf #16086)
-STM32 specific libs (cf #14188)

So we need to add a layer to the proposed hierarchy:

hal_stm32
└── stm32cube
    │   ├───  common
    │   │   ├─ stm32f1
    │   │   ├─ ...
    │   │   └─ stm32wb
    └─lib
        ├── stm32wb
        │       └─ hci
        └── ...

hal_st
└── stsensors
    │   ├───  stmemsc
    │   │   ├─ audio
    │   │   ├─ ....
    │   │   └─ proximity
    └─strf
        ├── subg1
        └── ...

With:

everything that supposed to run on STM32 in hal_stm32
everything that can run on any HW in hal_st

@avisconti, what is your feedback on this?

And finally, a last question for @nashif:

is use of hal_ namespace mandatory (since this is not only hals in there anymore)?

avisconti · 2019-05-16T12:03:14Z

everything that supposed to run on STM32 in hal_stm32

everything that can run on any HW in hal_st

Yes sure, but for the sake of precision I think that the hal_st is slightly wrong in the sense that stmemsc will cover only motion and environmental mems, leaving out proximity and audio which will be handled by different HALs. So picture may become:

But maybe, thining more on this, we can separate sensors with the RF stuff. Is it big enough to leave it in a separate repo?

erwango · 2019-05-16T12:09:18Z

But maybe, thining more on this, we can separate sensors with the RF stuff. Is it big enough to leave it in a separate repo?

This could be the way eventually, but for now, I'd prefer not spreading ST stuff too much.
I think that hal_st will hardly reach 1M so, I think it's not much of a trouble to keep inside one repo.

avisconti · 2019-05-16T12:14:05Z

But maybe, thining more on this, we can separate sensors with the RF stuff. Is it big enough to leave it in a separate repo?

This could be the way eventually, but for now, I'd prefer not spreading ST stuff too much.
I think that hal_st will hardly reach 1M so, I think it's not much of a trouble to keep inside one repo.

OK, fine.
I think we can keep the hal_st tree as I was saying. What do you think?

erwango · 2019-05-16T12:22:50Z

I think we can keep the hal_st tree as I was saying. What do you think?

Fine with me!

overheat · 2019-05-16T12:56:15Z

Hi, good to know there is a specific repo for ST stuff, cause ST has vast product lines, different kinds of sensors, wireless, audio and so on. It makes sense for users find them in one single repo.
But it should be a hard job for maintenance them. Thanks! @avisconti @erwango

erwango · 2019-05-16T13:00:20Z

@overheat , there will be 2 repos actually, one for eveything dedicated to STM32 MCUs (mostl hals) and one for ST components (sensors, rf and stuff), which should be lighter to pull for users not interested into the Cube HAL.

overheat · 2019-05-16T14:14:13Z

@overheat , there will be 2 repos actually, one for eveything dedicated to STM32 MCUs (mostl hals) and one for ST components (sensors, rf and stuff), which should be lighter to pull for users not interested into the Cube HAL.

@erwango ok, for me, it’s one for MCD, one for others. Also good for me, maybe also easier for maintenance.

_{Sent with GitHawk}

nashif · 2019-05-28T15:29:43Z

will open a new PR for this.

qmsi: move to a zephyr module

9a48fd7

Adds https://github.com/zephyrproject-rtos/hal_qmsi as a module managed by west. Signed-off-by: Anas Nashif <[email protected]>

nashif requested a review from erwango as a code owner May 9, 2019 17:51

nashif added the DNM This PR should not be merged (Do Not Merge) label May 9, 2019

ext: st: move ST HAL to its own repo

692b3ab

Move ST HAL out of the tree and use zephyr module under https://github.com/zephyrproject-rtos/hal_st instead. Signed-off-by: Anas Nashif <[email protected]>

nashif force-pushed the modularize_master_1 branch from 276a580 to 692b3ab Compare May 9, 2019 17:59

nashif requested a review from carlescufi May 9, 2019 18:04

nashif mentioned this pull request May 9, 2019

nxp: move HAL to external repo #16048

Closed

erwango mentioned this pull request May 22, 2019

ST modules organization #16316

Closed

nashif closed this May 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ext: st: move ST HAL to its own repo #16044

ext: st: move ST HAL to its own repo #16044

nashif commented May 9, 2019

erwango commented May 10, 2019

nashif commented May 10, 2019

erwango commented May 10, 2019

erwango commented May 10, 2019 •

edited

Loading

nashif commented May 10, 2019

erwango commented May 10, 2019

avisconti commented May 10, 2019 •

edited

Loading

nashif commented May 10, 2019

nashif commented May 10, 2019

erwango commented May 10, 2019 •

edited

Loading

avisconti commented May 10, 2019

erwango commented May 10, 2019

nashif commented May 10, 2019

erwango commented May 14, 2019

avisconti commented May 15, 2019

erwango commented May 16, 2019

avisconti commented May 16, 2019

erwango commented May 16, 2019

avisconti commented May 16, 2019

erwango commented May 16, 2019

overheat commented May 16, 2019

erwango commented May 16, 2019

overheat commented May 16, 2019

nashif commented May 28, 2019

ext: st: move ST HAL to its own repo #16044

ext: st: move ST HAL to its own repo #16044

Conversation

nashif commented May 9, 2019

erwango commented May 10, 2019

nashif commented May 10, 2019

erwango commented May 10, 2019

erwango commented May 10, 2019 • edited Loading

nashif commented May 10, 2019

erwango commented May 10, 2019

avisconti commented May 10, 2019 • edited Loading

nashif commented May 10, 2019

nashif commented May 10, 2019

erwango commented May 10, 2019 • edited Loading

avisconti commented May 10, 2019

erwango commented May 10, 2019

nashif commented May 10, 2019

erwango commented May 14, 2019

avisconti commented May 15, 2019

erwango commented May 16, 2019

avisconti commented May 16, 2019

erwango commented May 16, 2019

avisconti commented May 16, 2019

erwango commented May 16, 2019

overheat commented May 16, 2019

erwango commented May 16, 2019

overheat commented May 16, 2019

nashif commented May 28, 2019

erwango commented May 10, 2019 •

edited

Loading

avisconti commented May 10, 2019 •

edited

Loading

erwango commented May 10, 2019 •

edited

Loading