Hello Baruch, > > > How would a separate init_sensor routine improve things? > > > > So yes please do it, thanks to this you won't have to add the > > control_msb_offset member and can use a clean function. Moreover if > > in the future we see some usefulness for this LSB register then we > > could use the new compatible for the Armada 38x. > > There are two separate issues here: > > 1. DT binding > > 2. init_sensor callback implementation > > We both agree on #1. The A38x and CP110 need separate compatible > strings. In case we want to access the LSB control register on Armada > 38x, we will need yet another compatible string > (marvell,armada380-v2-thermal maybe?). > > As for #2, I'm all for sharing as much code as possible. I find the > vendor kernel approach of duplicating the init routines[1] unhelpful > as it violates the DRY principle. The differences between > armada380_init_sensor() and cp110_init_sensor() are minor. In my > opinion, these differences should be expressed explicitly in the > armada_thermal_data, in a similar way to my suggested > control_msb_offset field. The vendor code hides these differences in > slight variations of duplicated code. > > What is the advantage of a separate init routine? The advantage is that is the very near future I plan to add the overheat interrupt only on CP110 (not on 38x) and this needs some initialization. So if we don't make different routines now, I will have to do it right after. What would be fine is to have the shared code in a separate function, like it is done in Marvell kernel. What do you think about that? Thanks, Miquèl -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
