Cleared typedefs hiding unsigned long long type, to align with
kernel coding style.
Signed-off-by: Adrian Barnaś <abarnas@google.com>
---
.../pci/hive_isp_css_common/host/vmem.c | 42 +++++++++----------
1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/vmem.c b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/vmem.c
index 722b684fbc37..fd640e100591 100644
--- a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/vmem.c
+++ b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/vmem.c
@@ -13,35 +13,33 @@
#endif
#include "assert_support.h"
-typedef unsigned long long hive_uedge;
-typedef hive_uedge *hive_wide;
/* Copied from SDK: sim_semantics.c */
/* subword bits move like this: MSB[____xxxx____]LSB -> MSB[00000000xxxx]LSB */
-static inline hive_uedge
-subword(hive_uedge w, unsigned int start, unsigned int end)
+static inline unsigned long long
+subword(unsigned long long w, unsigned int start, unsigned int end)
{
return (w & (((1ULL << (end - 1)) - 1) << 1 | 1)) >> start;
}
/* inverse subword bits move like this: MSB[xxxx____xxxx]LSB -> MSB[xxxx0000xxxx]LSB */
-static inline hive_uedge
-inv_subword(hive_uedge w, unsigned int start, unsigned int end)
+static inline unsigned long long
+inv_subword(unsigned long long w, unsigned int start, unsigned int end)
{
return w & (~(((1ULL << (end - 1)) - 1) << 1 | 1) | ((1ULL << start) - 1));
}
-#define uedge_bits (8 * sizeof(hive_uedge))
+#define uedge_bits (8 * sizeof(unsigned long long))
#define move_lower_bits(target, target_bit, src, src_bit) move_subword(target, target_bit, src, 0, src_bit)
#define move_upper_bits(target, target_bit, src, src_bit) move_subword(target, target_bit, src, src_bit, uedge_bits)
#define move_word(target, target_bit, src) move_subword(target, target_bit, src, 0, uedge_bits)
static void
move_subword(
- hive_uedge *target,
+ unsigned long long *target,
unsigned int target_bit,
- hive_uedge src,
+ unsigned long long src,
unsigned int src_start,
unsigned int src_end)
{
@@ -49,18 +47,18 @@ move_subword(
unsigned int start_bit = target_bit % uedge_bits;
unsigned int subword_width = src_end - src_start;
- hive_uedge src_subword = subword(src, src_start, src_end);
+ unsigned long long src_subword = subword(src, src_start, src_end);
if (subword_width + start_bit > uedge_bits) { /* overlap */
- hive_uedge old_val1;
- hive_uedge old_val0 = inv_subword(target[start_elem], start_bit, uedge_bits);
+ unsigned long long old_val1;
+ unsigned long long old_val0 = inv_subword(target[start_elem], start_bit, uedge_bits);
target[start_elem] = old_val0 | (src_subword << start_bit);
old_val1 = inv_subword(target[start_elem + 1], 0,
subword_width + start_bit - uedge_bits);
target[start_elem + 1] = old_val1 | (src_subword >> (uedge_bits - start_bit));
} else {
- hive_uedge old_val = inv_subword(target[start_elem], start_bit,
+ unsigned long long old_val = inv_subword(target[start_elem], start_bit,
start_bit + subword_width);
target[start_elem] = old_val | (src_subword << start_bit);
@@ -69,8 +67,8 @@ move_subword(
static void
hive_sim_wide_unpack(
- hive_wide vector,
- hive_wide elem,
+ unsigned long long *vector,
+ unsigned long long *elem,
hive_uint elem_bits,
hive_uint index)
{
@@ -103,8 +101,8 @@ hive_sim_wide_unpack(
static void
hive_sim_wide_pack(
- hive_wide vector,
- hive_wide elem,
+ unsigned long long *vector,
+ unsigned long long *elem,
hive_uint elem_bits,
hive_uint index)
{
@@ -136,7 +134,7 @@ static void load_vector(
const t_vmem_elem *from)
{
unsigned int i;
- hive_uedge *data;
+ unsigned long long *data;
unsigned int size = sizeof(short) * ISP_NWAY;
VMEM_ARRAY(v, 2 * ISP_NWAY); /* Need 2 vectors to work around vmem hss bug */
@@ -146,9 +144,9 @@ static void load_vector(
#else
hrt_master_port_load(ISP_BAMEM_BASE[ID] + (unsigned long)from, &v[0][0], size);
#endif
- data = (hive_uedge *)v;
+ data = (unsigned long long *)v;
for (i = 0; i < ISP_NWAY; i++) {
- hive_uedge elem = 0;
+ unsigned long long elem = 0;
hive_sim_wide_unpack(data, &elem, ISP_VEC_ELEMBITS, i);
to[i] = elem;
@@ -166,10 +164,10 @@ static void store_vector(
VMEM_ARRAY(v, 2 * ISP_NWAY); /* Need 2 vectors to work around vmem hss bug */
//load_vector (&v[1][0], &to[ISP_NWAY]); /* Fetch the next vector, since it will be overwritten. */
- hive_uedge *data = (hive_uedge *)v;
+ unsigned long long *data = (unsigned long long *)v;
for (i = 0; i < ISP_NWAY; i++) {
- hive_sim_wide_pack(data, (hive_wide)&from[i], ISP_VEC_ELEMBITS, i);
+ hive_sim_wide_pack(data, (unsigned long long *)&from[i], ISP_VEC_ELEMBITS, i);
}
assert(ISP_BAMEM_BASE[ID] != (hrt_address) - 1);
#if !defined(HRT_MEMORY_ACCESS)
--
2.51.0.318.gd7df087d1a-googOn Mon, Sep 01, 2025 at 09:10:49AM +0000, Adrian Barnaś wrote:
> Cleared typedefs hiding unsigned long long type, to align with
> kernel coding style.
...
> -static inline hive_uedge
> -subword(hive_uedge w, unsigned int start, unsigned int end)
> +static inline unsigned long long
> +subword(unsigned long long w, unsigned int start, unsigned int end)
> {
> return (w & (((1ULL << (end - 1)) - 1) << 1 | 1)) >> start;
> }
>
> /* inverse subword bits move like this: MSB[xxxx____xxxx]LSB -> MSB[xxxx0000xxxx]LSB */
> -static inline hive_uedge
> -inv_subword(hive_uedge w, unsigned int start, unsigned int end)
> +static inline unsigned long long
> +inv_subword(unsigned long long w, unsigned int start, unsigned int end)
> {
> return w & (~(((1ULL << (end - 1)) - 1) << 1 | 1) | ((1ULL << start) - 1));
> }
Also consider to simplify the above (in a separate change).
static inline unsigned long long
subword(unsigned long long w, unsigned int start, unsigned int end)
{
return (w & GENMASK_ULL(end, 0)) >> start;
}
static inline unsigned long long
inv_subword(unsigned long long w, unsigned int start, unsigned int end)
{
return w & (~GENMASK_ULL(end, 0) | GENMASK_ULL(start, 0));
}
...if I'm not mistaken, so double check all these.
At least in my case the end == 64 is not allowed while it seems the original
code allows it to be equal to the end == 63 case. Needs testing anyway...
--
With Best Regards,
Andy Shevchenko
On Mon, Sep 1, 2025 at 2:35 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Mon, Sep 01, 2025 at 09:10:49AM +0000, Adrian Barnaś wrote:
> > Cleared typedefs hiding unsigned long long type, to align with
> > kernel coding style.
>
> ...
>
> > -static inline hive_uedge
> > -subword(hive_uedge w, unsigned int start, unsigned int end)
> > +static inline unsigned long long
> > +subword(unsigned long long w, unsigned int start, unsigned int end)
> > {
> > return (w & (((1ULL << (end - 1)) - 1) << 1 | 1)) >> start;
> > }
> >
> > /* inverse subword bits move like this: MSB[xxxx____xxxx]LSB -> MSB[xxxx0000xxxx]LSB */
> > -static inline hive_uedge
> > -inv_subword(hive_uedge w, unsigned int start, unsigned int end)
> > +static inline unsigned long long
> > +inv_subword(unsigned long long w, unsigned int start, unsigned int end)
> > {
> > return w & (~(((1ULL << (end - 1)) - 1) << 1 | 1) | ((1ULL << start) - 1));
> > }
>
> Also consider to simplify the above (in a separate change).
>
> static inline unsigned long long
> subword(unsigned long long w, unsigned int start, unsigned int end)
> {
> return (w & GENMASK_ULL(end, 0)) >> start;
> }
>
> static inline unsigned long long
> inv_subword(unsigned long long w, unsigned int start, unsigned int end)
> {
> return w & (~GENMASK_ULL(end, 0) | GENMASK_ULL(start, 0));
> }
>
> ...if I'm not mistaken, so double check all these.
>
> At least in my case the end == 64 is not allowed while it seems the original
> code allows it to be equal to the end == 63 case. Needs testing anyway...
Those functions works odd:
when (end = 8, start = 0) it affects bits 0...7
This should make the same results, will check twice if i not missed
anything and post v2:
static inline unsigned long long _subword(unsigned long long w,
unsigned int start,
unsigned int end)
{
return (w & GENMASK_ULL(end-1, 0)) >> start;
}
static inline unsigned long long _inv_subword(unsigned long long w,
unsigned int start,
unsigned int end)
{
return w & (~GENMASK_ULL(end-1, start));
}
Thank you for a review
Adrian Barnaś
On Mon, Sep 01, 2025 at 03:27:19PM +0200, Adrian Barnaś wrote:
> On Mon, Sep 1, 2025 at 2:35 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Mon, Sep 01, 2025 at 09:10:49AM +0000, Adrian Barnaś wrote:
...
> > > -static inline hive_uedge
> > > -subword(hive_uedge w, unsigned int start, unsigned int end)
> > > +static inline unsigned long long
> > > +subword(unsigned long long w, unsigned int start, unsigned int end)
> > > {
> > > return (w & (((1ULL << (end - 1)) - 1) << 1 | 1)) >> start;
> > > }
> > >
> > > /* inverse subword bits move like this: MSB[xxxx____xxxx]LSB -> MSB[xxxx0000xxxx]LSB */
> > > -static inline hive_uedge
> > > -inv_subword(hive_uedge w, unsigned int start, unsigned int end)
> > > +static inline unsigned long long
> > > +inv_subword(unsigned long long w, unsigned int start, unsigned int end)
> > > {
> > > return w & (~(((1ULL << (end - 1)) - 1) << 1 | 1) | ((1ULL << start) - 1));
> > > }
> >
> > Also consider to simplify the above (in a separate change).
> >
> > static inline unsigned long long
> > subword(unsigned long long w, unsigned int start, unsigned int end)
> > {
> > return (w & GENMASK_ULL(end, 0)) >> start;
> > }
> >
> > static inline unsigned long long
> > inv_subword(unsigned long long w, unsigned int start, unsigned int end)
> > {
> > return w & (~GENMASK_ULL(end, 0) | GENMASK_ULL(start, 0));
> > }
> >
> > ...if I'm not mistaken, so double check all these.
> >
> > At least in my case the end == 64 is not allowed while it seems the original
> > code allows it to be equal to the end == 63 case. Needs testing anyway...
>
> Those functions works odd:
> when (end = 8, start = 0) it affects bits 0...7
Yes, that's what I meant. But it does the same for 7, 0.
That's why every caller needs to be carefully checked for these corner cases.
64 is special because it will give complete garbage in my case.
I suspect they never extracting anything on the non-aligned byte borders.
> This should make the same results, will check twice if i not missed
> anything and post v2:
>
> static inline unsigned long long _subword(unsigned long long w,
> unsigned int start,
> unsigned int end)
> {
> return (w & GENMASK_ULL(end-1, 0)) >> start;
> }
>
> static inline unsigned long long _inv_subword(unsigned long long w,
> unsigned int start,
> unsigned int end)
> {
> return w & (~GENMASK_ULL(end-1, start));
> }
Maybe, but again needs to be carefully checked and tested.
You can propose a separate patch as RFC.
--
With Best Regards,
Andy Shevchenko
© 2016 - 2025 Red Hat, Inc.